Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Node Upgrade #30420

Merged
merged 3 commits into from
May 18, 2022
Merged

Node Upgrade #30420

merged 3 commits into from
May 18, 2022

Conversation

aht007
Copy link
Member

@aht007 aht007 commented May 17, 2022

Description

This PR adds node 16 support to edx-platform and also updates the dependent packages in package.json
Tested these changes by creating Sandbox with Node 16 configurations as well as on Sandbox with node 12 configuration.
Related PR to move prod to Node16 will be merged later on

@aht007
Copy link
Member Author

aht007 commented May 17, 2022

PR-30354 was merged in the same context yesterday but caused issues on production after which it was reverted. Turns out that it was using [email protected] in which there is a known empty build error due to which the required assets couldn't be found in studio and hence File & Uploads section wasn't visible. I have explicitly updated the version of studio-frontend now in this PR and also tested the changes on the sandbox to verify that the error doesn't exist now. Anyone who wants to confirm can visit this Sandbox.
Further details of the issue can also be found here

@aht007 aht007 requested review from a team and timmc-edx May 17, 2022 12:17
@aht007 aht007 force-pushed the aht007/Node-Upgrade branch from 5871edf to 2f62fc2 Compare May 18, 2022 10:03
@aht007 aht007 merged commit f801dea into master May 18, 2022
@aht007 aht007 deleted the aht007/Node-Upgrade branch May 18, 2022 11:12
@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

aht007 added a commit that referenced this pull request May 18, 2022
@aht007 aht007 mentioned this pull request May 18, 2022
@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

aht007 added a commit that referenced this pull request May 19, 2022
* feat: Node16 upgrade
@regisb
Copy link
Contributor

regisb commented May 24, 2022

This PR is breaking the package-lock.json file in multiple ways. As a consequence it is no longer possible to build the "nightly" image of Tutor.

Note that Tutor used npm clean-install both for performance and security/reliability reasons. See this PR: overhangio/tutor#666

"Host key verification failed"

When running npm clean-install we get:

npm ERR! /usr/bin/git ls-remote -h -t ssh://[email protected]/velesin/jasmine-jquery.git                                                                                                         
npm ERR! 
npm ERR! Host key verification failed.
npm ERR! fatal: Could not read from remote repository.
npm ERR! 
npm ERR! Please make sure you have the correct access rights
npm ERR! and the repository exists.

That's because the jasmine-jquery package was configured to install from ssh, and not https.

"Unsupported platform for fsevents"

After upgrading nodejs to 16.15.0 in the Docker image, I still cannot build the image:

npm ERR! code EBADPLATFORM
npm ERR! notsup Unsupported platform for [email protected]: wanted {"os":"darwin"} (current: {"os":"linux","arch":"x64"})
npm ERR! notsup Valid OS:    darwin
npm ERR! notsup Valid Arch:  undefined
npm ERR! notsup Actual OS:   linux
npm ERR! notsup Actual Arch: x64
npm verb exit 1
npm timing npm Completed in 2691ms
npm verb code 1

The reason for that is that I'm installing packages on Linux, and not macOS. Unfortunately I cannot link to the exact lines in package-lock.json because that file is too large to display in Github. You can have a look at its internals here: https://raw.githubusercontent.com/openedx/edx-platform/master/package-lock.json

@aht007
Copy link
Member Author

aht007 commented May 24, 2022

@regisb The host key verification issue is known to us and arises when trying to install packages with outdated Node versions on Docker only. For this either please update to Node16 or update npm to version 8 with Node12
For the unsupported platform issue, currently we have fixed it by using npm install for installing dependencies after a lengthy discussion which you can see here openedx-unsupported/configuration#6716
The Unsupported platform for fsevents issue is an internal issue to npm since it should be a warning in essence and shouldn't be installed on unsupported platforms but instead npm is raising error due to unknown reasons. A fix on our side would be to update the dependencies but that wasn't covered in this PR's domain and needs updates to codebase as well. Until this is not done I would recommend using npm install

@regisb
Copy link
Contributor

regisb commented May 24, 2022

Thanks for the quick reply @aht007, I appreciate it.

To be honest I do not have enough experience with npm to understand the technical aspect of the problem -- there are too many concepts which I do not understand in that conversation.

You're saying that we should resort to npm install -- I'm fine with that, but this can only be a temporary fix. We cannot use npm install as a long-term solution. That's because of the unpinned packages in edx-platform's package.json file.

So the question becomes: how long will we have to use npm install? In other words: what's the timeline to implement the "right solution" mentioned by @davidjoy here?

@aht007
Copy link
Member Author

aht007 commented May 25, 2022

I will create a ticket for that in frontend-bom's board and coordinate with them about the timeline for the fix

jawad-khan pushed a commit that referenced this pull request Jun 14, 2022
* feat: Node16 upgrade
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants