-
Notifications
You must be signed in to change notification settings - Fork 24
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
feat: node 18 support, crawlee and bullseye #83
Conversation
vladfrangu
commented
Aug 16, 2022
•
edited
Loading
edited
- Adds images for nodejs 18
- Upgrades all images that use debian to debian 11 bullseye (also means latest playwright works in them natively)
- updated all the test code to crawlee as well as adds it as a dependency to all images
- Closes fix: correctly sort compatible versions by major, then minor, and otherwise by locale #79
Right off the bat I can see that nodejs 14 and 15 are probably to be removed if we want to move to actor sdk + crawlee... Is that ok? Are we ready for that? |
why is working with linux so painful
I'd wait once people start using Node.js 18 more (a few weeks). Then there will be a drop-off in Node.js 14 users. |
# Cleanup apt caches | ||
RUN apt-get clean -y && apt-get autoremove -y \ | ||
&& rm -rf /var/lib/apt/lists/* \ | ||
&& rm -rf /src/*.deb | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's something we've done in all images to shave off some extra space from the redundant deb packages cached by apt (that said I never tried it with/without it so...maybe it's not? I could try it out and see if theres a size diff)
I would drop node < 16 right ahead. Why should we base this on usage of node 18? Also, the old images will be still there for people using v2. Using v3 requires node 16. |
this will also instantly fix this: nodejs/node#39400 and nodejs/node#32978 |
fyi node 18 images will be opt in before lts is out, we will keep node 16 in the templates |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Please test how template build times look on the platform with these images. The target is < 1 min.
the hardcoded |
Yep! That's just the default value if not specified! If we specify NODE_VERSION=18 in CI we'll build a node 18 image |