Handle import path change for vllm lora#376
Conversation
Summary of ChangesHello @Datta0, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a breaking change in the vLLM library where the import paths for LoRA modules were refactored. By introducing conditional import statements, the system can now seamlessly operate with different vLLM versions, enhancing the robustness and maintainability of the codebase without requiring manual adjustments for varying vLLM installations. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly handles a breaking change in vllm's LoRA module import paths by using a try-except block. This ensures backward compatibility with older versions of vllm. The implementation is clean and effective. I have one suggestion to fix a broken URL in a code comment to aid future maintenance.
| LRUCacheLoRAModelManager, create_lora_manager) | ||
| except ImportError: | ||
| # Newer vLLM version moved/split lora methods | ||
| # https://github.com/vllm-project/vllm/pull/30253 |
There was a problem hiding this comment.
The URL in this comment is broken and leads to a 404 error, which makes it difficult for future developers to understand the context for this change. The correct pull request appears to be #4701. Please update the link to improve code maintainability.
| # https://github.com/vllm-project/vllm/pull/30253 | |
| # https://github.com/vllm-project/vllm/pull/4701 |
This reverts commit 00a8d68.
* Update __init__.py * Update gradient_checkpointing.py * Update __init__.py * Update gradient_checkpointing.py * Update compiler.py * Handle import path change for vllm lora (#376) * Handle import path change for vllm lora * Better handle sleep and wakeup * Revert "Better handle sleep and wakeup" This reverts commit 00a8d68. * Update compiler.py --------- Co-authored-by: Datta Nimmaturi <venkatadattasainimmaturi@gmail.com>
2 participants
vLLM modified their LoRA modules and split up the paths. This is to address that
vllm-project/vllm#30253