Merge teams #117
Merge teams #117
Conversation
- Implemented a new endpoint to merge teams, allowing system administrators to merge a source team into a target team. - Added conflict resolution strategies: delete, rename, and cancel, to handle key name conflicts during the merge process. - Introduced helper functions for checking key name conflicts and migrating users and keys between teams. - Enhanced LiteLLMService to update key team associations as part of the merge process. - Developed comprehensive tests to validate the merge functionality, including scenarios for successful merges, unauthorized access, and conflict handling. - Updated schemas to include request and response models for team merging operations.
…ions - Added validation to prevent merging a source team with active product associations, returning a 400 error if such associations exist. - Updated the merge process documentation to reflect the new validation step. - Introduced a new test case to ensure the merge fails correctly when the source team has active product associations, verifying that both teams remain intact in this scenario.
- Introduced a new dialog for merging teams, allowing administrators to select a target and source team. - Implemented conflict resolution strategies for handling key name conflicts during the merge process. - Added state management for merging teams, including loading states and success/error notifications. - Enhanced UI with alerts to warn users about the permanent deletion of the source team post-merge. - Updated the merge functionality to ensure proper handling of team data and user migration.
- Updated the `isTeamExpired` function to clarify the conditions under which a team is considered expired, specifically focusing on the presence of active products. - Removed redundant checks related to active products in the UI rendering logic for expired teams. - Enhanced code readability and maintainability by streamlining the expiration determination process.
…sed on user role - Updated the `create_and_set_access_token` function to accept a user object, allowing for differentiated cookie expiration times: 30 minutes for regular users and 8 hours for system administrators. - Modified login and sign-in flows to pass the user object when creating access tokens. - Added comprehensive tests to verify cookie expiration behavior for both regular users and system administrators during login and sign-in processes.
app/api/teams.py
Outdated
| return remaining_keys | ||
| elif strategy == "rename": | ||
| # Rename conflicting keys in team2 | ||
| suffix = rename_suffix or "_merged" |
There was a problem hiding this comment.
probably not a real issue, but do we have to worry about the renamed key also having a conflict? like if the other team already had a key named "key1_merged"?
There was a problem hiding this comment.
Good catch - let me change the default to include the team it is coming from (which would be unique).
| current_user=current_user, | ||
| user_role="system_admin", # System admin context for merge operations | ||
| db=db | ||
| ) |
There was a problem hiding this comment.
is there any scenario where these get deleted but then for whatever reason the save/commit call fails when we actually do the migration and then the keys are gone?
There was a problem hiding this comment.
There is, but I don't have a good way to rollback in that case since this delete is fully destructive including tearing down vector DBs if they are part of the key. Given that the keys being deleted is the expected behaviour I'm OK with the risk.
app/api/teams.py
Outdated
| ).first() | ||
|
|
||
| if not region: | ||
| continue |
There was a problem hiding this comment.
when would this be the case? and is it a problem if there are relevant keys associated with an inactive region?
app/api/teams.py
Outdated
| try: | ||
| await litellm_service.update_key_team_association( | ||
| key.litellm_token, | ||
| f"{region.name.replace(' ', '_')}_{target_team.id}" |
There was a problem hiding this comment.
this is just like a magic string that is parsed to back out the associated team id?
app/api/teams.py
Outdated
| if user.team_id != target_team_id: | ||
| user.team_id = target_team_id | ||
| migrated_count += 1 | ||
| db.commit() |
There was a problem hiding this comment.
it might be fine but it seems like these interspersed commits could result in the DB getting into an inconsistent state if something fails partway through.
| # Re-raise HTTP exceptions as-is | ||
| raise | ||
| except Exception as e: | ||
| db.rollback() |
There was a problem hiding this comment.
will this actually work if commits have been made in the _migrate_keys_to_team and _migrate_users_to_team functions?
…team merges - Updated the `create_llm_token` function to utilize a new static method for formatting team IDs, improving consistency. - Simplified user and key migration logic in the team merge process by removing redundant helper functions and directly implementing the migration within the merge function. - Ensured changes are flushed to the database to persist user and key updates during team merges.
Uh oh!
There was an error while loading. Please reload this page.