-
Notifications
You must be signed in to change notification settings - Fork 605
chore: Finish vllm upgrade to 0.10.1 + cleanup #2528
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
WalkthroughBumps vLLM to 0.10.1 across config and packaging, updates FlashInf reference, adjusts install script defaults/help to reference variables, and updates a documentation URL in a Python comment. No functional code or control flow changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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: 2
🧹 Nitpick comments (1)
components/backends/vllm/src/dynamo/vllm/args.py (1)
212-216
: Assertion message references kv_port but checks side_channel_portThe assert checks side_channel_port is set, but the error message says “Must set the kv_port...”. This can mislead operators debugging misconfiguration.
Apply this diff to correct the message:
- assert ( - config.side_channel_port is not None - ), "Must set the kv_port, use configure_ports_with_etcd" + assert ( + config.side_channel_port is not None + ), "Must set side_channel_port, use configure_ports_with_etcd"
📜 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)
components/backends/vllm/src/dynamo/vllm/args.py
(1 hunks)container/Dockerfile.vllm
(1 hunks)container/deps/vllm/install_vllm.sh
(2 hunks)pyproject.toml
(1 hunks)
🔇 Additional comments (3)
pyproject.toml (1)
56-60
: Pin update to vLLM 0.10.1 looks goodThe optional dependency bump to vllm==0.10.1 aligns with the Dockerfile notes and install script changes. NIXL constraint remains <=0.4.1, consistent with the Dockerfile’s NIXL_REF=0.4.1.
components/backends/vllm/src/dynamo/vllm/args.py (1)
172-176
: Doc reference updated to v0.10.1The URL now points to the corresponding line in vLLM 0.10.1. No code behavior change.
container/deps/vllm/install_vllm.sh (1)
29-31
: FlashInfer default bumped to v0.2.11Default aligns with the Dockerfile ARG and PR objective. Looks good.
/ok to test 7118d27 |
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.
Examples are working for me, just approved MR. Two caveats:
-
lm_cache example doesn't work but this is known as we don't install it into the arm64 container
-
I'm not currently testing dsr1 as we haven't verified functionality in previous releases for arm64
/ok to test 10148a7 |
Signed-off-by: Hannah Zhang <[email protected]>
Overview:
Upgrade vllm in pyproject.toml and flashinfer version and update comments/references
This is a follow up on #2509
Details:
Where should the reviewer start?
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
Chores
Documentation
Refactor
Bug Fixes