-
Notifications
You must be signed in to change notification settings - Fork 686
chore: update nixl to 0.4.0 release #1860
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
WalkthroughThis update changes the NIXL dependency from version 0.3.1 to 0.4.0 across documentation, build scripts, and Rust dependencies. The commit hash for NIXL in the build script is updated, and the support matrix and Cargo.toml now reference the new version. No other logic or compatibility details are modified. Changes
Possibly related PRs
Poem
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
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: 0
🔭 Outside diff range comments (1)
container/build.sh (1)
117-121: Switch to using thev0.4.0tag for resilienceI’ve verified that
refs/tags/v0.4.0on https://github.com/ai-dynamo/nixl.git currently resolves to
3c47a48955e6f96bd5d4fb43a9d80bb64722f8e4, matching your hard-pinned SHA. To avoid future drift if the tag moves or is re-tagged, I recommend defaulting to the tag name and allowing an override:• In
container/build.sharound lines 117–121, replace:- NIXL_COMMIT=3c47a48955e6f96bd5d4fb43a9d80bb64722f8e4 + # Default to the v0.4.0 tag, but allow override for patch bumps + NIXL_COMMIT=${NIXL_COMMIT:-v0.4.0}• CI or deploy jobs can bump
NIXL_COMMITin one place for security patches.
• If you ever need to pin to a specific SHA, you still can by exportingNIXL_COMMITbeforehand.This keeps the script clear and makes minor version updates trivial.
🧹 Nitpick comments (3)
lib/llm/Cargo.toml (1)
88-89: Exact-pin to0.4.0may block security/bug-fix patch releasesAll other third-party crates here use semver-compatible version ranges (
"0.26","0.16", etc.). Suggest matching that style unless you have a reproducibility requirement:-nixl-sys = {version = "0.4.0", optional = true } +nixl-sys = {version = "0.4", optional = true }This will auto-pull
0.4.xbug-fixes without further PRs.</review_comment_end -->
docs/support_matrix.md (2)
70-77: Update footnotes when bumping major depsYou already bumped the NIXL version to 0.4.0 👍. The adjacent footnote still references ai-dynamo-vllm
0.8.4.post4and TensorRT-LLM1.0.0rc– confirm these are consistent with the container images produced bybuild.sh.</review_comment_end -->
84-86: Minor formatting: keep versions alignedFor readability, align the three-row block-dependency table:
-| **TensorRT-LLM** | 1.0.0rc² | -| **NIXL** | 0.4.0 | +| **TensorRT-LLM** | 1.0.0rc² | +| **NIXL** | 0.4.0 |(optional)
</review_comment_end -->
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locklib/bindings/python/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
container/build.sh(1 hunks)docs/support_matrix.md(1 hunks)lib/llm/Cargo.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kthui
PR: ai-dynamo/dynamo#1424
File: lib/runtime/src/pipeline/network/egress/push_router.rs:204-209
Timestamp: 2025-06-13T22:07:24.843Z
Learning: The codebase uses async-nats version 0.40, not the older nats crate. Error handling should use async_nats::error::Error variants, not nats::Error variants.
lib/llm/Cargo.toml (1)
Learnt from: kthui
PR: ai-dynamo/dynamo#1424
File: lib/runtime/src/pipeline/network/egress/push_router.rs:204-209
Timestamp: 2025-06-13T22:07:24.843Z
Learning: The codebase uses async-nats version 0.40, not the older nats crate. Error handling should use async_nats::error::Error variants, not nats::Error variants.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pre-merge-rust (lib/runtime/examples)
- GitHub Check: pre-merge-rust (.)
- GitHub Check: pre-merge-rust (lib/bindings/python)
- GitHub Check: Build and Test - vllm
🔇 Additional comments (1)
lib/llm/Cargo.toml (1)
33-37: Follow-up: audit for 0.4 API breakage
nixl-sysjumped a minor version – it is likely to have breaking C-ABI or feature-flag changes. Please make sure:
block_manager,testing-nixl, and production code still compile.- The Docker image’s
nixlcheckout (now 0.4.0) is built with matching Cargo features (e.g.kv-cacheflags changed names between 0.3 → 0.4).Consider a quick
cargo check --all-featuresin CI before merging.</review_comment_end -->
|
I think our builders or image need updating, possibly this nixl needs a newer glibc. I'm seeing this now in "Build and Test - vllm jobs: |
This reverts commit 5fa4cdd.
Overview:
update nixl build and crate version to 0.4.0 release
ref: OPS-367
Summary by CodeRabbit