-
Notifications
You must be signed in to change notification settings - Fork 7k
[deps][ci] compiling all depsets in single job #57957
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
[deps][ci] compiling all depsets in single job #57957
Conversation
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
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
The pull request combines multiple depset checks into a single job in the .buildkite/dependencies.rayci.yml file. This change aims to consolidate the dependency compilation process for raydepsets, improving efficiency and reducing redundancy in the CI pipeline. The review focuses on ensuring the correctness of the combined job and identifying any potential issues arising from the consolidation.
| commands: | ||
| - bazel run //ci/raydepsets:raydepsets -- build ci/raydepsets/configs/rayllm.depsets.yaml --check | ||
| - chown -R 2000:100 /artifact-mount | ||
| - cp ./python/deplocks/llm/* /artifact-mount/ | ||
| job_env: manylinux | ||
| depends_on: manylinux | ||
|
|
||
| - label: ":tapioca: build: raydepsets: compile ray img dependencies" | ||
| key: raydepsets_compile_rayimg_dependencies | ||
| tags: always | ||
| instance_type: medium | ||
| commands: | ||
| - bazel run //ci/raydepsets:raydepsets -- build ci/raydepsets/configs/rayimg.depsets.yaml --check | ||
| job_env: manylinux | ||
| depends_on: manylinux | ||
|
|
||
| - label: ":tapioca: build: raydepsets: compile multimodal inference release test dependencies" | ||
| key: raydepsets_compile_multimodal_inference_release_test_dependencies | ||
| tags: always | ||
| instance_type: medium | ||
| commands: | ||
| - bazel run //ci/raydepsets:raydepsets -- build ci/raydepsets/configs/release_multimodal_inference_benchmarks_tests.depsets.yaml --check |
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.
Consider consolidating these bazel run commands into a single command for better readability and maintainability. This can be achieved by chaining the commands with &&.
commands:
- bazel run //ci/raydepsets:raydepsets -- build ci/raydepsets/configs/rayllm.depsets.yaml --check && \
bazel run //ci/raydepsets:raydepsets -- build ci/raydepsets/configs/rayimg.depsets.yaml --check && \
bazel run //ci/raydepsets:raydepsets -- build ci/raydepsets/configs/release_multimodal_inference_benchmarks_tests.depsets.yaml --check| - label: ":tapioca: build: raydepsets: compile LLM dependencies" | ||
| key: raydepsets_compile_llm_dependencies | ||
| - label: ":tapioca: build: raydepsets: compile all dependencies" | ||
| key: raydepsets_compile_all_dependencies |
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.
maybe add the TODO line in the code here?
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
combining all depset checks into a single job TODO: add raydepset feature to build all depsets for the depset graph --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> Signed-off-by: xgui <xgui@anyscale.com>
combining all depset checks into a single job TODO: add raydepset feature to build all depsets for the depset graph --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
combining all depset checks into a single job TODO: add raydepset feature to build all depsets for the depset graph --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> Signed-off-by: Aydin Abiar <aydin@anyscale.com>
combining all depset checks into a single job
TODO: add raydepset feature to build all depsets for the depset graph