-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
fix(setup): improve precompiled wheel setup for Docker builds #22025
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
fix(setup): improve precompiled wheel setup for Docker builds #22025
Conversation
|
👋 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 🚀 |
5747076 to
a47c9a9
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 precompiled wheel handling for Docker builds, which is a good improvement for maintainability. The core logic is moved into a utility class, and the extraction process now happens before the main setup() call. I've identified a critical bug that would cause the setup script to crash and a high-severity resource leak from uncleaned temporary directories. Addressing these will make the build process more robust.
a47c9a9 to
a8518a8
Compare
This refactors the logic for handling `VLLM_USE_PRECOMPILED` to ensure that Docker builds extract only the required .so files and properly modify the package_data before setup() - Removes errant precompiled wheel copy that was copying old code - e.g. not code from the current checkout. - Moves precompiled wheel extraction logic into a utility class - Applies package_data patch before calling setup() - Now skips build_ext when precompiled is enabled - Supports fallback to nightly wheel if latest commit isn't available Follow-up to PR vllm-project#21964 as part of improving build times for CI. Signed-off-by: dougbtv <dosmith@redhat.com>
Head branch was pushed to by a user without write access
a8518a8 to
e7c9c50
Compare
|
@dougbtv @DarkLight1337 @simon-mo I think this broke the Python-only Installation Test https://buildkite.com/vllm/ci/builds/25598/steps/canvas?sid=01986167-695e-4a0e-b2a5-dd8aef0cb236 |
…vllm-project#22025)" This reverts commit 58bb902. Signed-off-by: Kebe <mail@kebe7jun.com>
…roject#22025) Signed-off-by: dougbtv <dosmith@redhat.com>
…roject#22025) Signed-off-by: dougbtv <dosmith@redhat.com> Signed-off-by: Jinzhen Lin <linjinzhen@hotmail.com>
…roject#22025) Signed-off-by: dougbtv <dosmith@redhat.com> Signed-off-by: Noam Gat <noamgat@gmail.com>
…roject#22025) Signed-off-by: dougbtv <dosmith@redhat.com> Signed-off-by: Paul Pak <paulpak58@gmail.com>
…roject#22025) Signed-off-by: dougbtv <dosmith@redhat.com> Signed-off-by: Diego-Castan <diego.castan@ibm.com>
…roject#22025) Signed-off-by: dougbtv <dosmith@redhat.com>
…roject#22025) Signed-off-by: dougbtv <dosmith@redhat.com>
This refactors the logic for handling
VLLM_USE_PRECOMPILEDto ensure that Docker builds extract only the required .so files and properly modify the package_data before setup()Follow-up to PR #21964 as part of improving build times for CI.
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.Purpose
Fixes errant wheel copy and improves the flow so that package_data is computed prior to setup.
Test Plan
Build and inspect on local work station.
Test Result
Previously, old code would be overwritten by the copied wheel.
Locally built and inspected file contents.
Realized that the package_data list was compiled too late, refactored the flow to account for it, doing everything up front before
setup()Wound up fixing it up and inspecting final build like so to look for a modification I put in place:
Found my marker in a modified file, e.g.
!bangas seen in print output above.(Optional) Documentation Update