-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
[Fix] Preserve key_alias and team_id metadata in /user/daily/activity/aggregated after key deletion or regeneration #20684
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
fef5c5f
fa95853
ef25ecb
6f49261
88a631d
db9b98c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3313,6 +3313,20 @@ async def regenerate_key_fn( | |
|
|
||
| verbose_proxy_logger.debug("key_in_db: %s", _key_in_db) | ||
|
|
||
| # Save the old key record to deleted table before regeneration | ||
| # This preserves key_alias and team_id metadata for historical spend records | ||
| try: | ||
| await _persist_deleted_verification_tokens( | ||
| keys=[_key_in_db], | ||
| prisma_client=prisma_client, | ||
| user_api_key_dict=user_api_key_dict, | ||
| litellm_changed_by=litellm_changed_by, | ||
| ) | ||
| except Exception: | ||
| verbose_proxy_logger.debug( | ||
| "Failed to persist old key record to deleted table during regeneration" | ||
| ) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Regeneration can lose history In |
||
|
|
||
| new_token = get_new_token(data=data) | ||
|
|
||
| new_token_hash = hash_token(new_token) | ||
|
|
@@ -3749,7 +3763,7 @@ async def list_keys( | |
| else: | ||
| admin_team_ids = None | ||
|
|
||
| if not user_id and user_api_key_dict.user_role not in [ | ||
| if user_id is None and user_api_key_dict.user_role not in [ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated to PR scope This fix ( Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does not change the flow.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where did this change come from? looks like you're about to cause a regression: #20623
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed it |
||
| LitellmUserRoles.PROXY_ADMIN.value, | ||
| LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY.value, | ||
| ]: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Broad exception hides bugs
get_api_key_metadata()swallows any exception when queryinglitellm_deletedverificationtokenand just returns partial metadata. This can silently mask real issues (e.g., query/schema errors) and lead tokey_alias/team_idunexpectedly staying null with no actionable signal. At minimum, log the exception object (or re-raise non-“table missing” errors) so production failures don’t get silently ignored.