mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-08 22:21:35 +00:00
fix(ci): apply Black formatting to 14 files and stabilize flaky caplog tests
- Run Black formatter on 14 files that were failing the lint check - Replace caplog-based assertions in TestAliasConflicts with unittest.mock.patch on verbose_logger.warning for xdist compatibility - The caplog fixture can produce empty text in pytest-xdist workers in certain CI environments, causing flaky test failures Co-authored-by: Ishaan Jaff <ishaan-jaff@users.noreply.github.com>
This commit is contained in:
parent
bd17d123c2
commit
1547362461
15 changed files with 85 additions and 60 deletions
|
|
@ -398,6 +398,7 @@ class LiteLLMResponsesTransformationHandler(CompletionTransformationBridge):
|
|||
ResponseOutputMessage,
|
||||
ResponseReasoningItem,
|
||||
)
|
||||
|
||||
try:
|
||||
from openai.types.responses.response_output_item import (
|
||||
ResponseApplyPatchToolCall,
|
||||
|
|
@ -460,7 +461,9 @@ class LiteLLMResponsesTransformationHandler(CompletionTransformationBridge):
|
|||
accumulated_tool_calls.append(tool_call_dict)
|
||||
tool_call_index += 1
|
||||
|
||||
elif ResponseApplyPatchToolCall is not None and isinstance(item, ResponseApplyPatchToolCall):
|
||||
elif ResponseApplyPatchToolCall is not None and isinstance(
|
||||
item, ResponseApplyPatchToolCall
|
||||
):
|
||||
from litellm.responses.litellm_completion_transformation.transformation import (
|
||||
LiteLLMCompletionResponsesConfig,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -2680,7 +2680,9 @@ def anthropic_messages_pt( # noqa: PLR0915
|
|||
_content_is_list = "content" in assistant_content_block and isinstance(
|
||||
assistant_content_block["content"], list
|
||||
)
|
||||
_content_list = assistant_content_block.get("content") if _content_is_list else None
|
||||
_content_list = (
|
||||
assistant_content_block.get("content") if _content_is_list else None
|
||||
)
|
||||
_list_has_thinking = False
|
||||
if _content_is_list and _content_list is not None:
|
||||
for _item in _content_list:
|
||||
|
|
|
|||
|
|
@ -79,7 +79,9 @@ class AnthropicFilesConfig(BaseFilesConfig):
|
|||
return AnthropicError(
|
||||
status_code=status_code,
|
||||
message=error_message,
|
||||
headers=cast(httpx.Headers, headers) if isinstance(headers, dict) else headers,
|
||||
headers=cast(httpx.Headers, headers)
|
||||
if isinstance(headers, dict)
|
||||
else headers,
|
||||
)
|
||||
|
||||
def validate_environment(
|
||||
|
|
|
|||
|
|
@ -144,9 +144,7 @@ class BaseModelResponseIterator:
|
|||
# Skip empty lines (common in SSE streams between events).
|
||||
# Only apply to str chunks — non-string objects (e.g. Pydantic
|
||||
# BaseModel events from the Responses API) must pass through.
|
||||
if isinstance(str_line, str) and (
|
||||
not str_line or not str_line.strip()
|
||||
):
|
||||
if isinstance(str_line, str) and (not str_line or not str_line.strip()):
|
||||
continue
|
||||
|
||||
# chunk is a str at this point
|
||||
|
|
@ -184,9 +182,7 @@ class BaseModelResponseIterator:
|
|||
# Skip empty lines (common in SSE streams between events).
|
||||
# Only apply to str chunks — non-string objects (e.g. Pydantic
|
||||
# BaseModel events from the Responses API) must pass through.
|
||||
if isinstance(str_line, str) and (
|
||||
not str_line or not str_line.strip()
|
||||
):
|
||||
if isinstance(str_line, str) and (not str_line or not str_line.strip()):
|
||||
continue
|
||||
|
||||
# chunk is a str at this point
|
||||
|
|
|
|||
|
|
@ -201,7 +201,9 @@ class BlackForestLabsImageEditConfig(BaseImageEditConfig):
|
|||
return image
|
||||
elif isinstance(image, list):
|
||||
# If it's a list, take the first image
|
||||
return self._read_image_bytes(image[0], depth=depth + 1, max_depth=max_depth)
|
||||
return self._read_image_bytes(
|
||||
image[0], depth=depth + 1, max_depth=max_depth
|
||||
)
|
||||
elif isinstance(image, str):
|
||||
if image.startswith(("http://", "https://")):
|
||||
# Download image from URL
|
||||
|
|
|
|||
|
|
@ -71,7 +71,9 @@ class PerplexityResponsesConfig(OpenAIResponsesAPIConfig):
|
|||
result: List[Any] = []
|
||||
for item in input:
|
||||
if isinstance(item, dict) and "type" not in item:
|
||||
new_item = dict(item) # convert to plain dict to avoid TypedDict checking
|
||||
new_item = dict(
|
||||
item
|
||||
) # convert to plain dict to avoid TypedDict checking
|
||||
new_item["type"] = "message"
|
||||
result.append(new_item)
|
||||
else:
|
||||
|
|
|
|||
|
|
@ -378,9 +378,7 @@ if MCP_AVAILABLE:
|
|||
# Resolve a server name to its UUID if needed
|
||||
_name_resolved = None
|
||||
if server_id not in allowed_server_ids:
|
||||
_name_resolved = global_mcp_server_manager.get_mcp_server_by_name(
|
||||
server_id
|
||||
)
|
||||
_name_resolved = global_mcp_server_manager.get_mcp_server_by_name(server_id)
|
||||
if _name_resolved is not None and _name_resolved.server_id in set(
|
||||
allowed_server_ids
|
||||
):
|
||||
|
|
@ -442,9 +440,7 @@ if MCP_AVAILABLE:
|
|||
extra_headers=user_oauth_extra_headers,
|
||||
)
|
||||
except Exception as e:
|
||||
verbose_logger.exception(
|
||||
f"Error getting tools from {server.name}: {e}"
|
||||
)
|
||||
verbose_logger.exception(f"Error getting tools from {server.name}: {e}")
|
||||
return {
|
||||
"tools": [],
|
||||
"error": "server_error",
|
||||
|
|
@ -473,7 +469,9 @@ if MCP_AVAILABLE:
|
|||
_name_resolved = None
|
||||
if server_id not in allowed_server_ids:
|
||||
_name_resolved = global_mcp_server_manager.get_mcp_server_by_name(server_id)
|
||||
if _name_resolved is not None and _name_resolved.server_id in set(allowed_server_ids):
|
||||
if _name_resolved is not None and _name_resolved.server_id in set(
|
||||
allowed_server_ids
|
||||
):
|
||||
server_id = _name_resolved.server_id
|
||||
|
||||
if server_id not in allowed_server_ids:
|
||||
|
|
@ -518,7 +516,9 @@ if MCP_AVAILABLE:
|
|||
server_auth_header = _get_server_auth_header(
|
||||
server, mcp_server_auth_headers, mcp_auth_header
|
||||
)
|
||||
user_oauth_extra_headers = await _get_user_oauth_extra_headers(server, user_api_key_dict)
|
||||
user_oauth_extra_headers = await _get_user_oauth_extra_headers(
|
||||
server, user_api_key_dict
|
||||
)
|
||||
|
||||
try:
|
||||
list_tools_result = await _get_tools_for_single_server(
|
||||
|
|
@ -529,9 +529,7 @@ if MCP_AVAILABLE:
|
|||
extra_headers=user_oauth_extra_headers,
|
||||
)
|
||||
except Exception as e:
|
||||
verbose_logger.exception(
|
||||
f"Error getting tools from {server.name}: {e}"
|
||||
)
|
||||
verbose_logger.exception(f"Error getting tools from {server.name}: {e}")
|
||||
return {
|
||||
"tools": [],
|
||||
"error": "server_error",
|
||||
|
|
@ -905,7 +903,9 @@ if MCP_AVAILABLE:
|
|||
try:
|
||||
client_id, client_secret, scopes = _extract_credentials(request)
|
||||
|
||||
_oauth2_flow: Optional[Literal["client_credentials", "authorization_code"]] = (
|
||||
_oauth2_flow: Optional[
|
||||
Literal["client_credentials", "authorization_code"]
|
||||
] = (
|
||||
"client_credentials"
|
||||
if client_id and client_secret and request.token_url
|
||||
else None
|
||||
|
|
|
|||
|
|
@ -64,7 +64,11 @@ from litellm.proxy.common_utils.http_parsing_utils import (
|
|||
populate_request_with_path_params,
|
||||
)
|
||||
from litellm.proxy.common_utils.realtime_utils import _realtime_request_body
|
||||
from litellm.proxy.utils import PrismaClient, ProxyLogging, normalize_route_for_root_path
|
||||
from litellm.proxy.utils import (
|
||||
PrismaClient,
|
||||
ProxyLogging,
|
||||
normalize_route_for_root_path,
|
||||
)
|
||||
from litellm.secret_managers.main import get_secret_bool
|
||||
from litellm.types.services import ServiceTypes
|
||||
|
||||
|
|
|
|||
|
|
@ -106,9 +106,13 @@ class _OPTIONAL_PresidioPIIMasking(CustomGuardrail):
|
|||
if (self.output_parse_pii or self.apply_to_output) and not logging_only:
|
||||
current_hook = self.event_hook
|
||||
if isinstance(current_hook, str) and current_hook != "post_call":
|
||||
self.event_hook = cast(List[GuardrailEventHooks], [current_hook, "post_call"])
|
||||
self.event_hook = cast(
|
||||
List[GuardrailEventHooks], [current_hook, "post_call"]
|
||||
)
|
||||
elif isinstance(current_hook, list) and "post_call" not in current_hook:
|
||||
self.event_hook = cast(List[GuardrailEventHooks], current_hook + ["post_call"])
|
||||
self.event_hook = cast(
|
||||
List[GuardrailEventHooks], current_hook + ["post_call"]
|
||||
)
|
||||
self.pii_entities_config: Dict[Union[PiiEntityType, str], PiiAction] = (
|
||||
pii_entities_config or {}
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1838,9 +1838,7 @@ async def _validate_update_key_data(
|
|||
|
||||
# Check team limits if key has a team_id (from request or existing key)
|
||||
team_obj: Optional[LiteLLM_TeamTableCachedObj] = None
|
||||
_team_id_to_check = data.team_id or getattr(
|
||||
existing_key_row, "team_id", None
|
||||
)
|
||||
_team_id_to_check = data.team_id or getattr(existing_key_row, "team_id", None)
|
||||
if _team_id_to_check is not None:
|
||||
team_obj = await get_team_object(
|
||||
team_id=_team_id_to_check,
|
||||
|
|
@ -1910,9 +1908,7 @@ async def _validate_update_key_data(
|
|||
if team_obj is None:
|
||||
raise HTTPException(
|
||||
status_code=500,
|
||||
detail={
|
||||
"error": "Team object not found for team change validation"
|
||||
},
|
||||
detail={"error": "Team object not found for team change validation"},
|
||||
)
|
||||
await validate_key_team_change(
|
||||
key=existing_key_row,
|
||||
|
|
|
|||
|
|
@ -846,12 +846,16 @@ async def get_generic_sso_response(
|
|||
verbose_proxy_logger.debug("calling generic_sso.verify_and_process")
|
||||
additional_generic_sso_headers_dict = _parse_generic_sso_headers()
|
||||
|
||||
code_verifier: Optional[str] = None # assigned inside try; initialized for type tracking
|
||||
code_verifier: Optional[
|
||||
str
|
||||
] = None # assigned inside try; initialized for type tracking
|
||||
|
||||
try:
|
||||
token_exchange_params = await SSOAuthenticationHandler.prepare_token_exchange_parameters(
|
||||
request=request,
|
||||
generic_include_client_id=generic_include_client_id,
|
||||
token_exchange_params = (
|
||||
await SSOAuthenticationHandler.prepare_token_exchange_parameters(
|
||||
request=request,
|
||||
generic_include_client_id=generic_include_client_id,
|
||||
)
|
||||
)
|
||||
|
||||
# Extract code_verifier (and the cache key for deferred deletion) before calling fastapi-sso
|
||||
|
|
@ -915,7 +919,9 @@ async def get_generic_sso_response(
|
|||
# Assign directly rather than relying on nonlocal mutation so that Pyright
|
||||
# can track that received_response is non-None from this point on.
|
||||
received_response = {
|
||||
k: v for k, v in combined_response.items() if k not in _OAUTH_TOKEN_FIELDS
|
||||
k: v
|
||||
for k, v in combined_response.items()
|
||||
if k not in _OAUTH_TOKEN_FIELDS
|
||||
}
|
||||
# In the PKCE path verify_and_process is skipped, so generic_sso.access_token
|
||||
# is never set. Read the token directly from the exchange response instead so
|
||||
|
|
@ -2598,7 +2604,9 @@ class SSOAuthenticationHandler:
|
|||
state,
|
||||
)
|
||||
else:
|
||||
verbose_proxy_logger.debug("PKCE code_verifier retrieved from cache")
|
||||
verbose_proxy_logger.debug(
|
||||
"PKCE code_verifier retrieved from cache"
|
||||
)
|
||||
elif isinstance(cached_data, str):
|
||||
# Handle legacy format (plain string) for backward compatibility
|
||||
code_verifier = cached_data
|
||||
|
|
@ -2647,7 +2655,9 @@ class SSOAuthenticationHandler:
|
|||
In strict mode (PKCE_STRICT_CACHE_MISS=true) raises ProxyException.
|
||||
Otherwise logs a warning and returns (token exchange proceeds without verifier).
|
||||
"""
|
||||
active_cache = redis_usage_cache if redis_usage_cache is not None else user_api_key_cache
|
||||
active_cache = (
|
||||
redis_usage_cache if redis_usage_cache is not None else user_api_key_cache
|
||||
)
|
||||
strict_cache_miss = (
|
||||
os.getenv("PKCE_STRICT_CACHE_MISS", "false").lower() == "true"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -5233,7 +5233,7 @@ def normalize_route_for_root_path(route: str) -> Optional[str]:
|
|||
root_path = get_server_root_path()
|
||||
if root_path and root_path != "/":
|
||||
if route.startswith(root_path + "/"):
|
||||
return route[len(root_path):]
|
||||
return route[len(root_path) :]
|
||||
return None
|
||||
return route
|
||||
|
||||
|
|
|
|||
|
|
@ -415,7 +415,9 @@ class LiteLLMCompletionResponsesConfig:
|
|||
if isinstance(new_msg, dict)
|
||||
else getattr(new_msg, "tool_calls", None)
|
||||
)
|
||||
new_tcs: list = _raw_tcs if isinstance(_raw_tcs, list) else []
|
||||
new_tcs: list = (
|
||||
_raw_tcs if isinstance(_raw_tcs, list) else []
|
||||
)
|
||||
for tc in new_tcs:
|
||||
LiteLLMCompletionResponsesConfig._add_tool_call_to_assistant(
|
||||
last_msg, tc
|
||||
|
|
|
|||
|
|
@ -35,9 +35,7 @@ def _is_valid_deployment_tag_regex(
|
|||
try:
|
||||
compiled = re.compile(pattern)
|
||||
except re.error:
|
||||
verbose_logger.warning(
|
||||
"tag_regex: invalid pattern %r — skipping", pattern
|
||||
)
|
||||
verbose_logger.warning("tag_regex: invalid pattern %r — skipping", pattern)
|
||||
continue
|
||||
for header_str in header_strings:
|
||||
if compiled.search(header_str):
|
||||
|
|
@ -109,12 +107,12 @@ def _match_deployment(
|
|||
# the regex to fire only when the deployment has NO plain tags, so we never
|
||||
# use regex as a backdoor around the operator's strict-tag policy.
|
||||
strict_tag_check_failed = (
|
||||
not match_any
|
||||
and bool(deployment_tags)
|
||||
and bool(request_tags)
|
||||
not match_any and bool(deployment_tags) and bool(request_tags)
|
||||
)
|
||||
if deployment_tag_regex and header_strings and not strict_tag_check_failed:
|
||||
regex_match = _is_valid_deployment_tag_regex(deployment_tag_regex, header_strings)
|
||||
regex_match = _is_valid_deployment_tag_regex(
|
||||
deployment_tag_regex, header_strings
|
||||
)
|
||||
if regex_match is not None:
|
||||
return {"matched_via": "tag_regex", "matched_value": regex_match}
|
||||
|
||||
|
|
@ -160,9 +158,7 @@ async def get_deployments_for_tag(
|
|||
# Build header strings for regex matching from what the proxy already stores.
|
||||
# Currently we match against User-Agent; format matches "^User-Agent: claude-code/..."
|
||||
user_agent = metadata.get("user_agent", "")
|
||||
header_strings: List[str] = (
|
||||
[f"User-Agent: {user_agent}"] if user_agent else []
|
||||
)
|
||||
header_strings: List[str] = [f"User-Agent: {user_agent}"] if user_agent else []
|
||||
|
||||
new_healthy_deployments: List[Any] = []
|
||||
default_deployments: List[Any] = []
|
||||
|
|
@ -173,8 +169,7 @@ async def get_deployments_for_tag(
|
|||
# User-Agent (all proxy requests do) but targets deployments with no
|
||||
# tag_regex will continue to use the original tag-only code path.
|
||||
has_regex_deployments = any(
|
||||
d.get("litellm_params", {}).get("tag_regex")
|
||||
for d in healthy_deployments
|
||||
d.get("litellm_params", {}).get("tag_regex") for d in healthy_deployments
|
||||
)
|
||||
has_tag_filter = bool(request_tags) or (
|
||||
bool(header_strings) and has_regex_deployments
|
||||
|
|
|
|||
|
|
@ -5,8 +5,9 @@ The ``_expand_model_aliases`` function processes ``aliases`` lists from model
|
|||
entries, creating shared dict references for alias entries at load time.
|
||||
"""
|
||||
|
||||
import logging
|
||||
from unittest.mock import patch
|
||||
|
||||
from litellm import verbose_logger
|
||||
from litellm.litellm_core_utils.get_model_cost_map import _expand_model_aliases
|
||||
|
||||
|
||||
|
|
@ -118,7 +119,7 @@ class TestExpandModelAliases:
|
|||
class TestAliasConflicts:
|
||||
"""Tests for alias conflict detection and handling."""
|
||||
|
||||
def test_alias_conflicts_with_canonical_entry(self, caplog):
|
||||
def test_alias_conflicts_with_canonical_entry(self):
|
||||
"""Alias that matches an existing canonical entry is skipped with a warning."""
|
||||
model_cost = {
|
||||
"model-latest": {
|
||||
|
|
@ -133,14 +134,17 @@ class TestAliasConflicts:
|
|||
"mode": "chat",
|
||||
},
|
||||
}
|
||||
with caplog.at_level(logging.WARNING, logger="LiteLLM"):
|
||||
with patch.object(verbose_logger, "warning") as mock_warn:
|
||||
result = _expand_model_aliases(model_cost)
|
||||
|
||||
# The canonical "model-dated" entry is preserved, not overwritten
|
||||
assert "model-dated" in result
|
||||
assert "alias conflict" in caplog.text.lower()
|
||||
# Verify a warning about the alias conflict was logged
|
||||
mock_warn.assert_called()
|
||||
warning_messages = " ".join(str(c) for c in mock_warn.call_args_list)
|
||||
assert "alias conflict" in warning_messages.lower()
|
||||
|
||||
def test_duplicate_alias_across_entries(self, caplog):
|
||||
def test_duplicate_alias_across_entries(self):
|
||||
"""Same alias claimed by two different entries: second one is skipped."""
|
||||
model_cost = {
|
||||
"model-a": {
|
||||
|
|
@ -156,13 +160,16 @@ class TestAliasConflicts:
|
|||
"mode": "chat",
|
||||
},
|
||||
}
|
||||
with caplog.at_level(logging.WARNING, logger="LiteLLM"):
|
||||
with patch.object(verbose_logger, "warning") as mock_warn:
|
||||
result = _expand_model_aliases(model_cost)
|
||||
|
||||
# "shared-alias" should point to model-a (first one wins)
|
||||
assert "shared-alias" in result
|
||||
assert result["shared-alias"]["input_cost_per_token"] == 1e-06
|
||||
assert "alias conflict" in caplog.text.lower()
|
||||
# Verify a warning about the alias conflict was logged
|
||||
mock_warn.assert_called()
|
||||
warning_messages = " ".join(str(c) for c in mock_warn.call_args_list)
|
||||
assert "alias conflict" in warning_messages.lower()
|
||||
|
||||
def test_canonical_entry_not_overwritten_by_alias(self):
|
||||
"""An alias must never overwrite an existing canonical entry's data."""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue