-
Notifications
You must be signed in to change notification settings - Fork 11
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
reduce config duplication, update third-party actions #101
Conversation
Thanks James! 🙏 Generally this seems reasonable to me. Asking Charles to take a closer look as he is the expert here 😉 |
I just reverted the Python 3.12 part of this. Realized that some of these images are actually installing RAPIDS packages, and we don't yet have packages for Python 3.12. e.g.: dask-build-environment/dask/environment.yml Lines 1 to 14 in e121aa8
The work to add Python 3.12 here is tracked in rapidsai/build-planning#40 |
Thanks @jameslamb! A lot of long-overdue improvements here.
Yeah this is definitely something that could use more visibility - this is echoed as part of the build process, but this is only visible in the Jenkins runs, which I think are only visible to
Modifying this for local testing on this work (picking a tag arbitrarily):
Where |
Ah got it, thanks for that! Yep, can confirm that I'm not able to view those Jenkins logs.
|
Unfortunately, there isn't a PR builder job, so checking that these changes pass CI can't be done until this is merged in 😕 and typically I've relied on local testing for more extensive changes. Locally, I can verify that not specifying all of the expected build args causes a failure:
Then confirmed for each project that including those build arguments allowed the builds to proceed and pass.
I noticed that we missed this explicit setting of
Would you mind applying this change? |
Followed up with an internal request to do this for now, if that spins off into a relevant GitHub issue can follow up here |
Ohhh ok, thanks for that. I thought that maybe I just couldn't see the PR builds because I didn't have sufficient permissions. Thanks for testing this one locally.
Oh cool yeah, looks like that would be safe to do. I dropped |
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.
Thanks @jameslamb!
Closes #100
Follow-up to #97
Contributes to rapidsai/build-planning#40
Proposes the following changes:
adding Python 3.12 images(now that those have been added upstream in Add Python 3.12 ci-imgs#137)ci/axis/dask.yaml
in the future for changes like "support a new Python version").gitignore
andREADME.md
Notes for Reviewers
How do I test this?
I can't figure out where these image builds actually happen. At https://github.com/rapidsai/dask-build-environment/actions, I only see the "Check for gpuCI Updates" job running.
Could someone please tell me where
docker build
based on theseDockerfiles
is run and how I could test these changes?