[BugFix][LoRA] use adapter_id instead of id field of lora_request#27728
[BugFix][LoRA] use adapter_id instead of id field of lora_request#27728jeejeelee merged 2 commits intovllm-project:mainfrom
Conversation
Signed-off-by: Biswa Panda <biswa.panda@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a bug in vllm/v1/core/block_pool.py. The code was attempting to access request.lora_request.id, but the LoRARequest object does not have an id attribute. The change replaces this with request.lora_request.adapter_id, which is the correct attribute. This prevents a runtime AttributeError when KV cache events are enabled for LoRA requests. The fix is accurate and addresses the issue effectively. I find no further issues with this change.
|
Agreed, this could have been caught with some tests. |
Probably |
Signed-off-by: Biswa Panda <biswa.panda@gmail.com>
|
Thanks @markmc , I've added unit test and results. PTAL |
lgtm, thanks! |
|
Thank you @markmc - when will this PR be merged? what are next steps? |
Next step is for a committer to review and approve the PR, perhaps @jeejeelee |
…lm-project#27728) Signed-off-by: Biswa Panda <biswa.panda@gmail.com>
…lm-project#27728) Signed-off-by: Biswa Panda <biswa.panda@gmail.com>
…lm-project#27728) Signed-off-by: Biswa Panda <biswa.panda@gmail.com>
…lm-project#27728) Signed-off-by: Biswa Panda <biswa.panda@gmail.com>
Purpose
This PR fixes a bug in the KV cache block storage system where the code was incorrectly trying to access
request.lora_request.idinstead of the correctrequest.lora_request.adapter_idproperty.Root Cause:
The LoRARequest class in vllm/lora/request.py does not have an
idfield. Instead, it has:lora_int_id: The actual ID fieldadapter_id: A property that returns lora_int_idSample Error:
Test Plan
Added unit test case
test_kv_cache_events_with_loraintest_prefix_caching.pyTest Result
After
Before
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.