-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Deprecate py27 (linux), add py39 (osx) #1336
Conversation
builds all passing except the 2 that require docker changes, so looks good |
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.
Looks good - will need a rebuild of the Docker images, and a full tagged build run to check if the wheel builds work.
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.
The Dockerfile
modifications in this file do not build - it appears as if the method on step 42 for acquiring nodejs
via yum
has bit-rotted, which I recognize was not touched by this PR, but nonetheless ...
The experience of discovering this after >1hr of docker build
, suggests strongly to me we need a better method of building Python, perhaps we build these images exclusively from Azure? Are these images needed still now that the pyarrow
dependency has been removed?
Regardless, two things are true, (1) I cannot merge this PR until the Dockerfile
builds, and (2) addressing this tech debt must pre-empt any future image dists.
I've updated and tested building the docker images from scratch. They are necessary as azure does not provide a |
(I've also dropped python2 in these last changes) |
Nice try - I've refactored these changes to #1374 |
missing flag fix copypasta
Upon further review, the Which means that |
Fixes #1334, adds py39 now that brew defaults this on OSX.