router: fix lora-only route rate limits#1237
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Code Review
This pull request updates the router to configure and delete rate limiters for LoRA adapters in addition to the base model, and adds a corresponding unit test. The reviewer identified a critical issue where unconditionally deleting rate limiters upon a ModelRoute deletion could leave other active routes sharing the same model or LoRA adapters unprotected, and suggested checking if the model or adapter is still registered in the store before deletion.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
3a20ba3 to
a4ac907
Compare
Signed-off-by: Avinash Kumar Deepak <avinash8655279@gmail.com>
a4ac907 to
c1a2352
Compare
|
Hey @hzxuzhonghu @LiZhenCheng9527 added a minimal fix for LoRA-only ModelRoute rate limits. Also covered the delete case so a limiter is not removed while another active rate-limited route still uses the same LoRA adapter. Please review when you get time. |
/kind bug
What this PR does / why we need it:
Fixes rate limits for LoRA-only ModelRoutes.
Previously the router registered the limiter only with
spec.modelName. For a LoRA-only route,modelNameis empty, so the limiter was added under an empty key and requests using the LoRA adapter name were not limited.This applies the same limiter to configured
loraAdapterstoo.Which issue(s) this PR fixes:
None
Special notes for your reviewer:
NONE
Does this PR introduce a user-facing change?: