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

Restore node 10 as base supported engine #349

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

minrk
Copy link
Member

@minrk minrk commented Sep 17, 2021

reverts commander to 7.x

closes #348 which has more info: bumping the minimum engine is a high-impact change, and our needs are not high. Since our primary target is JupyterHub deployments and not node developers, our target baseline should be widely available without resorting to nodesource. To me, that means at the very least, the version in the most recent Ubuntu LTS should be supported, and that's node 10 in ubuntu 20.04 at the moment.

@minrk minrk requested a review from consideRatio September 17, 2021 07:49
@meeseeksmachine
Copy link

This pull request has been mentioned on Jupyter Community Forum. There might be relevant details there:

https://discourse.jupyter.org/t/installing-jupyterhub-on-ubuntu-20-04-server/10808/3

@consideRatio
Copy link
Member

Yikes, I'm a bit hesitant on this because I know of intermittent failures in our CI workflows showing up specifically for node 10, and that I worry that we invite for additional maintenance of a hard to debug problem.

I think we should support running jupyterhub on ubuntu 20.04, but out of the box without installing a node that isn't already end of life? Hmmm...

Have you also seen the intermittent failures in our CI system related to node 10? I didn't investigate them more in detail as I knew it was to be dropped before.

@minrk
Copy link
Member Author

minrk commented Sep 17, 2021

node's "LTS" is not very "L" at all (2.5 years) which is barely the window between LTS releases. We don't require any recent node features, so I don't think we should be aggressive about dropping support for versions that work.

If node had better support for dropping old engine versions, I'd support tracking node LTS instead, but since all they have now is a completely unhelpful warning with no obvious action to take, and for our users, CHP is often the only node package they ever install, I think we should prioritize compatibility for users in realistic environments.

This is doubly frustrating now, because everything actually works fine with node 10, so the warning is purely a source of confusion and CHP works just fine on node 10, and the 'right' course of action for a user with node 10 is to just ignore the warning that CHP claims it is not compatible with node 10.

Flaky tests are frustrating, but if investigating the flakiness is not worth the necessary time, I think re-running flaky tests is preferable to dropping support for the latest widely available node version in our primary target distro.

@consideRatio consideRatio merged commit 8347036 into jupyterhub:main Sep 17, 2021
@consideRatio
Copy link
Member

Thanks for providing some additional background for this decision! Let's go for it!

My worry isn't the test failure itself as much as I'm worried about always thinking that anyone reporting a bug could use node 10 and that the failure was a runtime failure that was related to the flakey test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

npm engine requirement isn't taken into account at install time
3 participants