-
Notifications
You must be signed in to change notification settings - Fork 766
fix: remove kvmanager feature from python 3.12 ai-dynamo-runtime wheel #2456
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
WalkthroughRemoved the block-manager feature from maturin wheel builds in container Dockerfiles and switched Python package versioning to a fixed 0.4.0.post0, updating the root dependency to match. Other build steps and conditionals remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Docker as Docker Build
participant Maturin as maturin
participant Registry as Wheel Artifact
Dev->>Docker: Build image (Dockerfile/ Dockerfile.vllm)
Docker->>Maturin: build (no --features block-manager)
Maturin-->>Registry: Produce wheel with default features
Docker-->>Dev: Image with wheel
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
pyproject.toml (1)
28-28: Pinning ai-dynamo-runtime to 0.4.0.post0 keeps packages in lockstepGiven the tight coupling between ai-dynamo and ai-dynamo-runtime, this explicit pin is reasonable for the wheel change.
If you expect to publish multiple post releases quickly, consider a compatible pin (e.g., ~=0.4.0.post0) to reduce churn in the root package when only runtime is patched.
container/Dockerfile (1)
231-236: Add --no-default-features to all maturin builds to avoid enabling KVBM / bundling libnixlI ran the inspection script but no .whl files were found under dist, so I couldn't confirm wheel contents or GLIBC symbols — add this flag defensively and re-run the wheel check after building.
- File needing change:
- container/Dockerfile — around lines 231–236
Apply:
- maturin build --release --out /opt/dynamo/dist && \ + maturin build --release --no-default-features --out /opt/dynamo/dist && \ if [ "$RELEASE_BUILD" = "true" ]; then \ # do not enable KVBM feature, ensure compatibility with lower glibc - uv run --python 3.11 maturin build --release --out /opt/dynamo/dist && \ - uv run --python 3.10 maturin build --release --out /opt/dynamo/dist; \ + uv run --python 3.11 maturin build --release --no-default-features --out /opt/dynamo/dist && \ + uv run --python 3.10 maturin build --release --no-default-features --out /opt/dynamo/dist; \ fiPlease re-run the wheel-inspection script (or provide dist/*.whl) so we can verify no libnixl is bundled and the wheel targets the expected manylinux tag.
container/Dockerfile.vllm (1)
371-376: Good: maturin builds omit KVBM; also pass--no-default-featuresfor robustnessSame rationale as the main Dockerfile — explicitly disable default features for all maturin invocations to prevent accidental inclusion if defaults change.
Apply:
- maturin build --release --out /workspace/dist && \ + maturin build --release --no-default-features --out /workspace/dist && \ if [ "$RELEASE_BUILD" = "true" ]; then \ # do not enable KVBM feature, ensure compatibility with lower glibc - uv run --python 3.11 maturin build --release --out /workspace/dist && \ - uv run --python 3.10 maturin build --release --out /workspace/dist; \ + uv run --python 3.11 maturin build --release --no-default-features --out /workspace/dist && \ + uv run --python 3.10 maturin build --release --no-default-features --out /workspace/dist; \ fiNote: Earlier in this file, the workspace cargo build is invoked with
--features dynamo-llm/block-manager(Line 364 in context). That’s fine for producing binaries likemetrics, but it won’t affect maturin’s feature set as long as maturin explicitly disables defaults as recommended above.Use the same wheel inspection script suggested for the main Dockerfile to ensure no nixl artifacts and GLIBC_2.28 targeting.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
container/Dockerfile(1 hunks)container/Dockerfile.vllm(1 hunks)lib/bindings/python/pyproject.toml(1 hunks)pyproject.toml(2 hunks)
🔇 Additional comments (1)
pyproject.toml (1)
18-18: Approve — Python packages bumped to 0.4.0.post0; Rust/Cargo remains 0.4.0Brief: pyproject files were updated to 0.4.0.post0 while Cargo.toml (workspace + local crates) remains 0.4.0. This is expected (Cargo uses semver and doesn't accept PEP 440 post-release suffixes). Ensure tags/releases are created consistently so downstream tooling picks the intended post-release.
Files to note:
- pyproject.toml — version = "0.4.0.post0"
- lib/bindings/python/pyproject.toml — version = "0.4.0.post0"
- Cargo.toml — workspace.package version = "0.4.0" (and local crates: dynamo-runtime / dynamo-llm / dynamo-tokens = "0.4.0"; async_zmq dependency = "0.4.0")
#2456) Signed-off-by: Hannah Zhang <[email protected]>
Overview:
This removes the KVBM from the
ai-dynamo-runtimewheel so that in environments where the glibc version is not 2.38 (such as AWS AL2023) we are able to run dynamo in environments for python 3.12.Details:
Enabling the KVBM feature in the
ai-dynamo-runtimewheel requires packaginglibnixl.so. Unfortunately, due to lack of the publicly-available NVIDIA containers which contain both CUDA and a manylinux build environment there is not an easy way to buildlibnixl.soin such a way that the glibc version is kept equivalent to the 2.28 reported by the wheel.By removing it the wheel no longer contains
libnixl.soand has a glibc dependency of 2.28.Where should the reviewer start?
After pulling this branch in the dynamo repo run the following to build the container and copy out the wheels into a folder called
./wheelhouse:container/build.sh --framework none --tag dynamo:base-aws docker create --name aws dynamo:base-aws docker cp aws:/opt/dynamo/wheelhouse . docker rm awsIn the top-level of the dynamo directory add the following file named
Containerfile.al2023We will now build this container
docker build -f Containerfile.al2023 --tag al2023:p312-vllm .To test the container we will bring up Dynamo in the normal way using Qwen3-0.6B
Wait for a moment while the model is downloaded and the service starts. If you issue the command too early just wait a second and try again.
Once you have observed functionality you can clean up your running containers, delete the
./wheelhouse/folder and./Containerfile.al2023Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Fixes OPS-623
Summary by CodeRabbit