-
-
Notifications
You must be signed in to change notification settings - Fork 10k
[CI] Cleanup modelscope version constraint in Dockerfile #21243
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
Conversation
Signed-off-by: Kay Yan <kay.yan@daocloud.io>
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
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 cleans up a version constraint for the modelscope
package in the Dockerfiles, which is a good simplification. However, by removing the constraint entirely, the package is now unpinned. To ensure build reproducibility and prevent future breakages from new modelscope
releases, I've recommended pinning the dependency to a known-good version or a safe version range.
@@ -510,7 +510,7 @@ RUN --mount=type=cache,target=/root/.cache/uv \ | |||
else \ | |||
BITSANDBYTES_VERSION="0.46.1"; \ | |||
fi; \ | |||
uv pip install --system accelerate hf_transfer 'modelscope!=1.15.0' "bitsandbytes>=${BITSANDBYTES_VERSION}" 'timm==0.9.10' boto3 runai-model-streamer runai-model-streamer[s3] | |||
uv pip install --system accelerate hf_transfer modelscope "bitsandbytes>=${BITSANDBYTES_VERSION}" 'timm==0.9.10' boto3 runai-model-streamer runai-model-streamer[s3] |
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.
While removing the constraint for modelscope==1.15.0
is a good cleanup, leaving modelscope
completely unpinned in a production Dockerfile can lead to non-reproducible builds and potential breakages in the future if a new version introduces breaking changes.
To improve stability, it's best practice to pin dependencies. Could you please consider pinning modelscope
to a known good version range? For example, if versions greater than 1.15.0 are known to be stable, you could use a lower bound like >=1.15.1
. This would prevent the problematic version while still allowing updates, but in a more controlled way. An even safer approach would be to pin to a specific known-good version, e.g., ==1.16.0
.
uv pip install --system accelerate hf_transfer 'modelscope>=1.15.1' "bitsandbytes>=${BITSANDBYTES_VERSION}" 'timm==0.9.10' boto3 runai-model-streamer runai-model-streamer[s3]
@@ -47,7 +47,7 @@ FROM vllm-base AS vllm-openai | |||
|
|||
# install additional dependencies for openai api server | |||
RUN --mount=type=cache,target=/root/.cache/pip \ | |||
pip install accelerate hf_transfer pytest 'modelscope!=1.15.0' | |||
pip install accelerate hf_transfer pytest modelscope |
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.
Similar to the other Dockerfile, leaving modelscope
unpinned here could affect the stability and reproducibility of the build. A future release of modelscope
with breaking changes could cause this build to fail.
To ensure the Docker image remains stable over time, could you pin modelscope
to a known good version or at least a safe version range, for example, >=1.15.1
?
pip install accelerate hf_transfer pytest 'modelscope>=1.15.1'
…t#21243) Signed-off-by: Kay Yan <kay.yan@daocloud.io>
…t#21243) Signed-off-by: Kay Yan <kay.yan@daocloud.io> Signed-off-by: avigny <47987522+avigny@users.noreply.github.com>
…t#21243) Signed-off-by: Kay Yan <kay.yan@daocloud.io> Signed-off-by: x22x22 <wadeking@qq.com>
…t#21243) Signed-off-by: Kay Yan <kay.yan@daocloud.io>
…t#21243) Signed-off-by: Kay Yan <kay.yan@daocloud.io>
…t#21243) Signed-off-by: Kay Yan <kay.yan@daocloud.io> Signed-off-by: Jinzhen Lin <linjinzhen@hotmail.com>
…t#21243) Signed-off-by: Kay Yan <kay.yan@daocloud.io> Signed-off-by: Paul Pak <paulpak58@gmail.com>
…t#21243) Signed-off-by: Kay Yan <kay.yan@daocloud.io>
…t#21243) Signed-off-by: Kay Yan <kay.yan@daocloud.io> Signed-off-by: Diego-Castan <diego.castan@ibm.com>
…t#21243) Signed-off-by: Kay Yan <kay.yan@daocloud.io>
…t#21243) Signed-off-by: Kay Yan <kay.yan@daocloud.io>
Essential Elements of an Effective PR Description Checklist
supported_models.md
andexamples
for a new model.Purpose
Remove the
modelscope!=1.15.0
version constraint from Docker build configurations.This constraint was originally added in #5668 to resolve an issue specific to modelscope 1.15.0.
It can now be safely removed to simplify the code and improve readability because the issue has been addressed in newer releases.
Test Plan
Test Result
(Optional) Documentation Update