Fix duplicate teams in list_team endpoint #12142
Open
Conversation
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
- Modified config/update endpoint to always merge callbacks instead of overwriting - Fixed issue where UI-added callbacks would replace config-based callbacks - Added comprehensive tests for callback merging behavior - Handles edge cases: empty lists, non-list values, and deduplication Fixes BerriAI#12118
…en user.teams and team.members_with_roles - Use set to track team IDs and prevent duplicates when filtering teams - Check both user.teams and team.members_with_roles to handle data drift - Add warning logs when data drift is detected between the two sources - Add comprehensive test coverage for duplicate prevention - Ensure all teams are returned even if data sources are out of sync
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
colesmcintosh
changed the title
- Remove user.teams check as it could cause authorization issues - Keep team.members_with_roles as the single source of truth - Maintain duplicate prevention using set - Address review feedback about not using user.teams for filtering
- Fix mypy type errors in _filter_teams_by_user function - Support both Member objects and dict representations from database - Ensures compatibility with different data formats from Prisma
4 tasks
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.
Title
Fix duplicate teams in list_team endpoint
Relevant issues
Addresses review comments from #12082
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unitType
🐛 Bug Fix
Changes
This PR addresses the review comment from #12082 about duplicate teams in the list_team endpoint.
Fixed duplicate teams in list_team logic
members_with_rolestest_list_team_no_duplicates_in_fallbackto verify the fix_filter_teams_by_userto satisfy linting requirementsImportant Note on the Second Review Comment
The second review comment from krrishdholakia correctly pointed out that using
user.teamsas the source of truth (as done in #12082) could cause authorization issues, since other parts of the codebase rely onteam.members_with_rolesfor access checks.This PR reverts to using only
team.members_with_rolesas the single source of truth, which maintains consistency with the rest of the codebase. The root cause of any drift betweenuser.teamsandteam.members_with_rolesshould be addressed separately by ensuring these fields stay in sync during team member add/remove operations.Test Results
All team endpoint tests passing:
Technical Details
set()to efficiently track added team IDs and prevent duplicatesteam.members_with_rolesas the authoritative source