-
Notifications
You must be signed in to change notification settings - Fork 7k
[ci] fix serve minbuild #58762
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
[ci] fix serve minbuild #58762
Conversation
|
@abrarsheikh this will change the base from common py3.9 base to multipy base, and probably will fix the npm not found issue. |
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 correctly updates the CI configuration to make the minbuild process multi-python aware. The changes are consistent across the affected rayci.yml files and the wanda.yaml build definition, and the logic appears sound. I have one suggestion to improve the maintainability of the min.build.wanda.yaml file by reducing duplication.
ci/docker/min.build.wanda.yaml
Outdated
| build_args: | ||
| - PYTHON_VERSION | ||
| - EXTRA_DEPENDENCY | ||
| - DOCKER_IMAGE_BASE_BUILD=cr.ray.io/rayproject/oss-ci-base_build-py$PYTHON_VERSION |
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.
This line duplicates the base image URL from the froms field on line 2. To improve maintainability and avoid potential inconsistencies, consider defining this URL only once. If the wanda tool supports it, you could use a YAML anchor to define the URL in one place and reference it in both froms and build_args.
.buildkite/ml.rayci.yml
Outdated
| label: "wanda: minbuild-ml-py{{matrix}}" | ||
| wanda: ci/docker/min.build.wanda.yaml | ||
| depends_on: oss-ci-base_build | ||
| depends_on: oss-ci-base_build-multipy |
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.
Bug: ML Group: Undeclared Build Dependency
The minbuild-ml step depends on oss-ci-base_build-multipy, but this dependency is not declared at the group level. The ml group's top-level depends_on should include oss-ci-base_build-multipy to ensure the build is available when minbuild-ml attempts to use it, matching the pattern in core.rayci.yml and serve.rayci.yml.
.buildkite/ml.rayci.yml
Outdated
| label: "wanda: minbuild-ml-py{{matrix}}" | ||
| wanda: ci/docker/min.build.wanda.yaml | ||
| depends_on: oss-ci-base_build | ||
| depends_on: oss-ci-base_build-multipy |
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.
Bug: ML Group: Standardize Build Dependencies
The ml group's top-level depends_on is missing oss-ci-base_build-multipy, even though the minbuild-ml step depends on it. This is inconsistent with the core and serve pipeline groups, which declare this dependency at the group level. The ml group should include oss-ci-base_build-multipy in its top-level depends_on to ensure proper dependency ordering and availability.
6dc6324 to
e111abf
Compare
e111abf to
01b1725
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.
Bug: minbuild-serve missing dependency on base multipy build
The minbuild-serve build step is missing a depends_on: oss-ci-base_build-multipy field. The min.build.wanda.yaml was updated to use multi-Python base images, and minbuild-ml was updated to explicitly depend on oss-ci-base_build-multipy. For consistency and correctness, minbuild-serve should have the same explicit dependency since it uses the same min.build.wanda.yaml template.
.buildkite/serve.rayci.yml#L26-L37
ray/.buildkite/serve.rayci.yml
Lines 26 to 37 in 01b1725
| - name: minbuild-serve | |
| label: "wanda: minbuild-{{matrix.extra}}-py{{matrix.python}}" | |
| wanda: ci/docker/min.build.wanda.yaml | |
| matrix: | |
| setup: | |
| python: ["3.10"] | |
| extra: ["serve", "default"] | |
| env: | |
| PYTHON_VERSION: "{{matrix.python}}" | |
| EXTRA_DEPENDENCY: "{{matrix.extra}}" | |
| tags: cibase |
add `-f` flags, so that they will not fail silently Signed-off-by: Lonnie Liu <[email protected]>
69891be to
4209d32
Compare
fix serve minbuild ci base image Signed-off-by: Lonnie Liu <[email protected]>
fix serve minbuild ci base image Signed-off-by: Lonnie Liu <[email protected]>
adding `-f` flag in curl's, so that failure can be properly captured. Signed-off-by: Lonnie Liu <[email protected]> Signed-off-by: Aydin Abiar <[email protected]>
adding `-f` flag in curl's, so that failure can be properly captured. Signed-off-by: Lonnie Liu <[email protected]>
adding `-f` flag in curl's, so that failure can be properly captured. Signed-off-by: Lonnie Liu <[email protected]> Signed-off-by: YK <[email protected]>
adding `-f` flag in curl's, so that failure can be properly captured. Signed-off-by: Lonnie Liu <[email protected]>
adding
-fflag in curl's, so that failure can be properly captured.