fix: adapt the import path change for ToT vLLM#7749
fix: adapt the import path change for ToT vLLM#7749richardhuo-nv wants to merge 1 commit intomainfrom
Conversation
WalkthroughThe PR updates import statements across nine modules to support multiple vLLM versions. Direct imports of Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches⚔️ Resolve merge conflicts
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. Comment |
fix fix mm
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/src/dynamo/vllm/multimodal_utils/protocol.py`:
- Around line 32-38: The vLLM compatibility try/except import for
MultiModalUUIDDict and TokensPrompt is currently below first-party imports and
violates isort/ruff ordering; move the entire try/except block (the imports of
MultiModalUUIDDict and TokensPrompt) so it appears before any first-party
dynamo.* imports in protocol.py, ensuring third-party vllm imports come before
first-party imports and preserve the fallback ImportError behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7ec0b77d-b993-44b9-9e11-d3a830a7791e
📒 Files selected for processing (9)
components/src/dynamo/frontend/vllm_processor.pycomponents/src/dynamo/vllm/multimodal_handlers/multimodal_pd_worker_handler.pycomponents/src/dynamo/vllm/multimodal_handlers/worker_handler.pycomponents/src/dynamo/vllm/multimodal_utils/chat_processor.pycomponents/src/dynamo/vllm/multimodal_utils/protocol.pycomponents/src/dynamo/vllm/tests/test_vllm_renderer_api.pyexamples/multimodal/components/worker.pyexamples/multimodal/utils/chat_processor.pyexamples/multimodal/utils/protocol.py
9a8eac2 to
d71c51d
Compare
|
I don't think we should do this. That Dynamo targets a specific vllm release, to avoid having so many different code paths. We also test with only the version we target, because again that would be very complex. |
Overview:
Adapt the import path change for ToT vLLM, with a fallback to the old import path for older vLLM versions.
Details:
This vLLM PR (vllm-project/vllm#35182) changed the inputs path in vLLM. Update the import path on our end to ensure we stay compatible with these changes during the vLLM version upgrade, while still falling back to the old import path for older versions.
Where should the reviewer start?
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
Refactor
Chores