-
Notifications
You must be signed in to change notification settings - Fork 7.2k
[ci] Add wanda anyscale image builds for release tests #59937
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
base: andrew/revup/master/wanda-image-upload
Are you sure you want to change the base?
[ci] Add wanda anyscale image builds for release tests #59937
Conversation
|
f104bf5 to
288cadb
Compare
f963b42 to
0710971
Compare
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.
Code Review
This pull request refactors the Anyscale image build process to use Wanda for building and a new Python script for pushing images to multiple registries. The changes include new Wanda configuration files, the push_anyscale_image.py script, and updates to the Buildkite pipeline. My review identifies a few critical issues that will break the build, including a missing file in a Bazel target and an incomplete matrix configuration in the pipeline. I've also provided several suggestions to improve maintainability by reducing code duplication and making parts of the new script less brittle.
|
|
||
| # GPU_PLATFORM is the default GPU platform that gets aliased as "gpu" | ||
| # This must match the definition in ci/ray_ci/docker_container.py | ||
| GPU_PLATFORM = "cu12.1.1-cudnn8" |
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 GPU_PLATFORM is hardcoded here. The comment mentions it must match a definition in another file, which makes this brittle. If the default GPU platform changes, this script will require a manual update, which can be easily missed. Consider sourcing this value from a shared configuration file or passing it as a command-line argument to make the script more robust and maintainable.
| def _format_platform_tag(platform: str) -> str: | ||
| """Format platform as -cpu or shortened CUDA version.""" | ||
| if platform == "cpu": | ||
| return "-cpu" | ||
| # cu12.3.2-cudnn9 -> -cu123 | ||
| versions = platform.split(".") | ||
| return f"-{versions[0]}{versions[1]}" |
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 logic for parsing the CUDA version string using platform.split('.') is fragile. It assumes a specific format with dots as separators and might fail with an IndexError if the platform string format changes (e.g., cu12-cudnn8). Using a regular expression would be more robust for extracting the version parts. For example: re.match(r"cu(\\d+)\\.(\\d+)", platform).
0710971 to
4cdb2dd
Compare
46cc862 to
70df756
Compare
4cdb2dd to
f65669a
Compare
70df756 to
ae32d45
Compare
f65669a to
1141897
Compare
ae32d45 to
04cfd26
Compare
af3b600 to
6bbeb9e
Compare
04cfd26 to
55d1473
Compare
6bbeb9e to
1f049e8
Compare
55d1473 to
84d7479
Compare
84d7479 to
c275fe2
Compare
d43ccd7 to
8980d9c
Compare
7c78fa2 to
29cf5ab
Compare
8980d9c to
4a143b7
Compare
29cf5ab to
d566c8a
Compare
4a143b7 to
9243672
Compare
d566c8a to
9190029
Compare
b0970de to
f9436df
Compare
efba4b0 to
cba45eb
Compare
f9436df to
9f28339
Compare
cba45eb to
ba9c860
Compare
ba9c860 to
fc730b0
Compare
9f28339 to
8eb71d4
Compare
8eb71d4 to
99d9b0b
Compare
fc730b0 to
9373b7b
Compare
99d9b0b to
dcd2620
Compare
9373b7b to
1609eac
Compare
e4ba419 to
bfbedb4
Compare
1609eac to
7fe9ae2
Compare
bfbedb4 to
3fc8d89
Compare
3fc8d89 to
2becfe0
Compare
7fe9ae2 to
0ec84ea
Compare
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.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
2becfe0 to
c61e365
Compare
This adds wanda-based builds for anyscale images used in release tests. Changes: - Add ray-anyscale-cpu/cuda.wanda.yaml for ray anyscale images - Add ray-llm-anyscale-cuda.wanda.yaml for LLM images - Add ray-ml-anyscale-cuda.wanda.yaml for ML images - Add push_anyscale_image.py for pushing to ECR/GCP/Azure via crane - Update release/build.rayci.yml with build and push steps Topic: anyscale-image Relative: wanda-image-upload Signed-off-by: andrew <andrew@anyscale.com>
0ec84ea to
ea2ca72
Compare
c61e365 to
436f981
Compare
This adds wanda-based builds for anyscale images used in release tests.
Changes:
Topic: anyscale-image
Relative: wanda-image-upload
Signed-off-by: andrew andrew@anyscale.com