Skip to content

Conversation

michaelfeil
Copy link
Contributor

@michaelfeil michaelfeil commented Oct 13, 2025

Overview:

Details:

Where should the reviewer start?

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • closes GitHub issue: #xxx

Summary by CodeRabbit

  • Chores
    • Broadened wheel selection in container builds to install ai_dynamo_runtime using a version-agnostic pattern, improving compatibility across Python versions and reducing build fragility across all runtime variants.
    • Enabled ABI3 (py310) for Python bindings, enhancing cross-version compatibility and easing distribution of prebuilt wheels.
    • Overall, container images and Python wheels should be more resilient to Python minor version differences, resulting in smoother builds and deployments without changing runtime behavior.

Copy link

copy-pr-bot bot commented Oct 13, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Copy link

👋 Hi michaelfeil! Thank you for contributing to ai-dynamo/dynamo.

Just a reminder: The NVIDIA Test Github Validation CI runs an essential subset of the testing framework to quickly catch errors.Your PR reviewers may elect to test the changes comprehensively before approving your changes.

🚀

@github-actions github-actions bot added feat external-contribution Pull request is from an external contributor labels Oct 13, 2025
Signed-off-by: michaelfeil <[email protected]>
Signed-off-by: michaelfeil <[email protected]>
@michaelfeil michaelfeil marked this pull request as ready for review October 13, 2025 04:42
@michaelfeil michaelfeil requested review from a team as code owners October 13, 2025 04:42
Copy link
Contributor

coderabbitai bot commented Oct 13, 2025

Walkthrough

Broadened the ai_dynamo_runtime wheel selection from cp312-specific to any matching wheel across multiple Dockerfiles. Enabled the pyo3 abi3-py310 feature in the Python bindings Cargo.toml to build against a stable Python ABI. No public API changes.

Changes

Cohort / File(s) Summary of Changes
Docker build wheel glob update
container/Dockerfile, container/Dockerfile.sglang, container/Dockerfile.trtllm, container/Dockerfile.vllm
Relaxed pip install pattern from ai_dynamo_runtime*cp312*.whl to ai_dynamo_runtime*.whl to allow non-cp312 wheels; no other build-step changes.
Python bindings ABI config
lib/bindings/python/Cargo.toml
Enabled abi3-py310 feature for pyo3, switching to Python ABI3 targeting 3.10+. No dependency list or API surface changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nudge the wheels, wide paths I choose,
From cp312’s snug, to any shoes.
With abi3 I lightly hop,
Py 3.10’s rock becomes my stop.
Containers hum, the builds align—
A rabbit’s tweak, now running fine. 🐇✨

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The description is largely incomplete because the Overview section is blank, the Details section provides only a brief note, and the “Where should the reviewer start?” section is missing any file references. It fails to follow the required template structure and does not offer enough context for reviewers to understand the full scope of changes. Please fill in the Overview with a concise summary of what this PR accomplishes, expand the Details to describe each modification (including the Dockerfile and Cargo.toml changes), specify relevant files or areas for review under “Where should the reviewer start?”, and confirm the actual issue number in the Related Issues section.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly conveys that this PR introduces a feature for building the Python package with ABI support targeting Python 3.10 and above, which matches the primary change of enabling abi3-py310 in the build configuration. It is concise, specific to the main update, and avoids unnecessary details.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 90dc758 and 3e93858.

📒 Files selected for processing (5)
  • container/Dockerfile (1 hunks)
  • container/Dockerfile.sglang (1 hunks)
  • container/Dockerfile.trtllm (1 hunks)
  • container/Dockerfile.vllm (1 hunks)
  • lib/bindings/python/Cargo.toml (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: dmitry-tokarev-nv
PR: ai-dynamo/dynamo#2300
File: pyproject.toml:64-66
Timestamp: 2025-08-05T22:51:59.230Z
Learning: The ai-dynamo/dynamo project does not ship ARM64 wheels, so platform markers to restrict dependencies to x86_64 are not needed in pyproject.toml dependencies.
🔇 Additional comments (5)
container/Dockerfile.vllm (1)

251-253: Runtime wheel glob now matches abi3 builds

Switching to ai_dynamo_runtime*.whl ensures the new abi3-tagged runtime wheel gets installed; the prior cp312 suffix would have missed it. Change looks good.

container/Dockerfile.sglang (1)

203-205: SGLang image picks up abi3 runtime wheel

Matching pattern to ai_dynamo_runtime*.whl keeps this image aligned with the abi3 runtime artifact. Looks solid.

container/Dockerfile (1)

381-383: Dev image installs the abi3 runtime wheel

Broader glob is the right move so the abi3 wheel produced by maturin is consumed in dev images too. 👍

container/Dockerfile.trtllm (1)

238-240: TRT-LLM runtime stays in sync with abi3 packaging

ai_dynamo_runtime*.whl ensures this container grabs the new abi3-tagged runtime wheel; no issues spotted.

lib/bindings/python/Cargo.toml (1)

55-64: abi3 feature matches wheel strategy

Enabling abi3-py310 lines up with the new wheel distribution plan and our Python ≥3.10 support range. No issues here.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@michaelfeil
Copy link
Contributor Author

michaelfeil commented Oct 13, 2025

Okay, this is a good as is. @kthui (or anyone else?) can you kick of the ok to test magic here?

@rmccorm4
Copy link
Contributor

/ok to test 3e93858

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external-contribution Pull request is from an external contributor feat size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants