fix: fetch litellm user rows from /v2/user/info - #197
Draft
jlav wants to merge 2 commits into
Draft
Conversation
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HUMAN:
AGENT:
Why
Login provisioning calls litellm's
GET /user/infoduring the OAuth callback. That endpoint inlines every team the user belongs to, member rosters included. Every user gets added to one shared team, so the response is currently 10.3 MiB, and 99.89% of it is that team's 99,100-entry member list.The call takes around 5s and httpx aborts it at its 5s default. 40-50% of them are failing right now. That surfaces as a 500 on
/oauth/keycloak/callback, leaving a keycloak account with no OpenHandsUserrow. The payload grows about 26 KiB/day with signups, so it degrades on its own./v2/user/infoanswers with the user row on its own. Keys and team objects are left out.Summary
_get_userreads from/v2/user/infoand returns the flat user row, orNoneon a 404.migrate_entriesconsumes that row directly._user_existscalls_get_userrather than issuing its own/user/inforequest.Issue Number
Nothing filed in this repo. This came out of the activation-rate drop investigation tracked elsewhere as #638.
How to Test
Unit tests:
The new
TestGetUsercases cover the v2 request, an unknown user, and a 500.To confirm the v2 route is reachable through the ingress and see the size difference, against any real user id:
End to end, signing in with a fresh keycloak account on staging should show
GET /v2/user/infoin the litellm access logs whereGET /user/infoused to be, at roughly a kilobyte instead of ten megabytes.Video/Screenshots
n/a
Type
Notes
/v2/user/infoneeds litellm 1.84 or newer. The proxy is on 1.94.1._get_all_keys_for_userstill uses/user/info, since v2 omits keys. It runs on settings save rather than login, so it's not behind the callback 500s, but it does pull the same payload./key/list?return_full_object=trueis the follow-up. It needs pagination handling (sizedefaults to 10, max 100) and an exact user id match, because for a proxy-admin key litellm treatsuser_idas a case-insensitivecontains.Separately: the
LITELLM_MANAGEMENT_TIMEOUTwiring from #189 shipped in 1.53.0. Before that commit the provisioning client passed notimeout=at all, which is where the 5s ceiling came from. Independent of this PR, but it explains the 5002ms wall in the write-up.Enterprise server image for this PR: