mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
fix(router): address remaining Greptile review comments
- Cache LITELLM_ENABLE_TEAM_STALE_ALIAS_BYPASS at module level to avoid hot-path secret lookups - Add clarifying comments for should_include_deployment team isolation logic - Add negative assertion for update_team.assert_not_called() in test - Add docstring clarification for _get_team_deployments helper pattern - Add explicit assertion message in test_get_model_list_alias_optimization Made-with: Cursor
This commit is contained in:
parent
592ac98ddc
commit
2321d77599
5 changed files with 27 additions and 10 deletions
|
|
@ -41,6 +41,8 @@ service_logger_obj = ServiceLogging() # used for tracking latency on OTEL
|
|||
# Bounded dedup for stale-alias warnings (FIFO eviction when over cap).
|
||||
_MAX_STALE_ALIAS_WARNING_KEYS = 10_000
|
||||
_STALE_TEAM_ALIAS_WARNING_KEYS: OrderedDict[str, None] = OrderedDict()
|
||||
# Cache the stale alias bypass flag at module load to avoid hot-path secret lookups
|
||||
_ENABLE_TEAM_STALE_ALIAS_BYPASS: Optional[bool] = None
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
|
|
@ -1320,9 +1322,13 @@ def _update_model_if_team_alias_exists(
|
|||
|
||||
# Optional bypass for stale aliases from pre-PR deployments:
|
||||
# only enabled via feature flag to preserve backwards compatibility.
|
||||
enable_stale_alias_bypass = get_secret_bool(
|
||||
"LITELLM_ENABLE_TEAM_STALE_ALIAS_BYPASS", False
|
||||
)
|
||||
# Cached at module level to avoid hot-path secret lookups on every request.
|
||||
global _ENABLE_TEAM_STALE_ALIAS_BYPASS
|
||||
if _ENABLE_TEAM_STALE_ALIAS_BYPASS is None:
|
||||
_ENABLE_TEAM_STALE_ALIAS_BYPASS = get_secret_bool(
|
||||
"LITELLM_ENABLE_TEAM_STALE_ALIAS_BYPASS", False
|
||||
)
|
||||
enable_stale_alias_bypass = _ENABLE_TEAM_STALE_ALIAS_BYPASS
|
||||
# Check if the alias points to a team-scoped UUID name
|
||||
# (format: "model_name_{team_id}_{uuid}")
|
||||
is_stale_team_alias = aliased_target.startswith(
|
||||
|
|
|
|||
|
|
@ -470,6 +470,10 @@ async def _get_team_deployments(
|
|||
Fetch all deployments for a given team_id from the database.
|
||||
|
||||
Centralizes team deployment queries to ensure consistent filtering and error handling.
|
||||
This is the established helper pattern for team deployment DB access in this module.
|
||||
|
||||
Note: Direct Prisma call is intentional here as this IS the helper function that
|
||||
encapsulates the DB access pattern for team deployments.
|
||||
"""
|
||||
response = await prisma_client.db.litellm_proxymodeltable.find_many(
|
||||
where={
|
||||
|
|
|
|||
|
|
@ -8236,13 +8236,16 @@ class Router:
|
|||
):
|
||||
return True
|
||||
elif model_name is not None and model["model_name"] == model_name:
|
||||
# Fallback: check by internal model_name for non-team deployments
|
||||
# or deployments that haven't been migrated to team_public_model_name yet
|
||||
model_team_id = (model.get("model_info") or {}).get("team_id")
|
||||
if (
|
||||
team_id is None
|
||||
team_id is None # requester has no team constraint
|
||||
or model_team_id is None # global deployment - accessible to all teams
|
||||
or model_team_id == team_id
|
||||
or model_team_id == team_id # deployment belongs to requester's team
|
||||
):
|
||||
return True
|
||||
# No match: deployment is for a different team or doesn't match the requested model
|
||||
return False
|
||||
|
||||
def _get_all_deployments(
|
||||
|
|
|
|||
|
|
@ -44,7 +44,7 @@ def test_map_team_model_should_not_iterate_aliases_for_non_alias_team_model_name
|
|||
{f"alias-{idx}": "gpt-4" for idx in range(200)}
|
||||
)
|
||||
|
||||
assert (
|
||||
router.map_team_model(team_model_name="team-model", team_id="team-1")
|
||||
== "team-model"
|
||||
)
|
||||
# map_team_model should return the public name unchanged (not the internal UUID name)
|
||||
# so the router can find all sibling deployments via team_id filtering
|
||||
result = router.map_team_model(team_model_name="team-model", team_id="team-1")
|
||||
assert result == "team-model", f"Expected public name 'team-model', got {result}"
|
||||
|
|
|
|||
|
|
@ -802,7 +802,9 @@ class TestTeamModelUpdate:
|
|||
True,
|
||||
), patch(
|
||||
"litellm.proxy.management_endpoints.model_management_endpoints.team_model_add"
|
||||
) as mock_team_model_add:
|
||||
) as mock_team_model_add, patch(
|
||||
"litellm.proxy.management_endpoints.model_management_endpoints.update_team"
|
||||
) as mock_update_team:
|
||||
result = await _update_team_model_in_db(
|
||||
db_model=db_model,
|
||||
patch_data=patch_data,
|
||||
|
|
@ -814,6 +816,8 @@ class TestTeamModelUpdate:
|
|||
assert "team_public_model_name" in str(result.get("model_info", ""))
|
||||
# team_model_add must be called to add public name to team's models list
|
||||
mock_team_model_add.assert_called_once()
|
||||
# update_team (model_aliases write) must NOT be called in the new implementation
|
||||
mock_update_team.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rename_preserves_old_name_when_siblings_exist(self):
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue