[vllm] fix: LoRAModel import path change for vLLM 0.13.0#4631
[vllm] fix: LoRAModel import path change for vLLM 0.13.0#4631vermouth1992 merged 1 commit intoverl-project:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request addresses an ImportError for LoRAModel due to a path change in a recent vllm version by removing the import and the corresponding type hint. While this resolves the immediate issue, I've suggested an alternative approach that preserves the benefits of static type checking. My recommendations involve using a conditional import with typing.TYPE_CHECKING to handle different vllm versions gracefully. This allows restoring the LoRAModel type hint, which improves code clarity and maintainability without causing runtime errors.
74ac85d to
03decd1
Compare
There was a problem hiding this comment.
Code Review
This pull request updates the import path for LoRAModel to support a newer version of vLLM. The changes correctly use a try-except block for backward compatibility. However, the current implementation places the import within a TYPE_CHECKING block, making LoRAModel unavailable at runtime. This is a brittle approach that could lead to future runtime errors. I have provided suggestions to refactor this into a more robust, module-level import, which is a standard pattern for handling such compatibility issues.
Related to vllm-project/vllm#30253 (comment) `LoRAModel` import path was changed from `vllm.lora.models` to `vllm.lora.lora_model`. Signed-off-by: Hollow Man <hollowman@opensuse.org>
03decd1 to
e52318d
Compare
What does this PR do?
Related to vllm-project/vllm#30253 (comment)
LoRAModelimport path was changed fromvllm.lora.modelstovllm.lora.lora_model.Checklist Before Starting
[{modules}] {type}: {description}(This will be checked by the CI){modules}includefsdp,megatron,sglang,vllm,rollout,trainer,ci,training_utils,recipe,hardware,deployment,ray,worker,single_controller,misc,perf,model,algo,env,tool,ckpt,doc,data,cfg,reward,like[megatron, fsdp, doc]{type}is infeat,fix,refactor,chore,test[BREAKING]to the beginning of the title.[BREAKING][fsdp, megatron] feat: dynamic batchingTest
API and Usage Example
# Add code snippet or script demonstrating how to use thisDesign & Code Changes
Checklist Before Submitting
Important
Please check all the following items before requesting a review, otherwise the reviewer might deprioritize this PR for review.
pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=alwaysci-requestchannel in theverlSlack workspace. (If not accessible, please try the Feishu group (飞书群).)