[ci] fix BUILDKITE_BAZEL_CACHE_URL scope in manylinux-retag#62758
[ci] fix BUILDKITE_BAZEL_CACHE_URL scope in manylinux-retag#62758aslonnie wants to merge 2 commits into
Conversation
declare ARG below FROM and explicitly re-set ENV from the ARG value, so the rayturbo build_arg takes effect in the RUN and /home/forge/.bazelrc is written with the correct cache URL. Signed-off-by: Lonnie Liu <95255098+aslonnie@users.noreply.github.com>
declare ARG below FROM and explicitly re-set ENV from the ARG value, so the rayturbo build_arg takes effect in the RUN and /home/forge/.bazelrc is written with the correct cache URL. Signed-off-by: Lonnie Liu <95255098+aslonnie@users.noreply.github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the manylinux-retag.Dockerfile by moving the BUILDKITE_BAZEL_CACHE_URL argument after the FROM instruction and adding an ENV declaration. The review feedback correctly identifies that the ENV instruction is redundant because ARG values are already available during the build stage, and persisting this variable in the image metadata is unnecessary since it is already baked into the configuration files or provided at runtime.
| ARG BUILDKITE_BAZEL_CACHE_URL | ||
| ENV BUILDKITE_BAZEL_CACHE_URL=${BUILDKITE_BAZEL_CACHE_URL} |
There was a problem hiding this comment.
The ENV instruction on line 14 is redundant. In Docker, ARG values are automatically available as environment variables during the execution of RUN commands within the same build stage, so the ENV line is not needed for the subsequent RUN block to work.
Furthermore, using ENV persists the build-time value into the image's runtime environment. This is unnecessary here because:
- The
RUNblock already bakes the cache URL directly into the.bazelrcfile. - The CI runner (
ci/ray_ci/container.py) explicitly passes the correct environment variable at runtime, which would override this image-level default anyway.
Removing the ENV line keeps the image metadata cleaner and avoids baking in a potentially stale default URL.
ARG BUILDKITE_BAZEL_CACHE_URL
|
prefers #62723 |
Pull request was closed
2 participants
declare ARG below FROM and explicitly re-set ENV from the ARG value, so the rayturbo build_arg takes effect in the RUN and /home/forge/.bazelrc is written with the correct cache URL.