mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-25 01:02:15 +00:00
fix(proxy): report a stored alerting value as db even when it is null
A stored null or empty list for a nested alerting field is still the value the proxy serves when the config file leaves alerting_args alone, so the source is db. Keying off the value rather than its presence reported those fields as default and hid a stored setting that is genuinely in effect. Presence in the stored row now decides, with the config file still checked first so a config-owned key keeps reporting config. Test helpers are typed and the router test injects a stub rather than patching a class attribute.
This commit is contained in:
parent
f998ab53d5
commit
4602376977
4 changed files with 64 additions and 29 deletions
|
|
@ -15993,16 +15993,13 @@ def _nested_setting_source(
|
|||
field_name: str,
|
||||
field_default: JsonValue,
|
||||
) -> FieldSource:
|
||||
unset_source: Final[FieldSource] = "default" if field_default is not None else "unset"
|
||||
parent_value: Final = settings.config_value(parent_key)
|
||||
if isinstance(parent_value, Mapping) and field_name in parent_value:
|
||||
return "config"
|
||||
unset_source: Final[FieldSource] = "default" if field_default is not None else "unset"
|
||||
if settings.owned_by_config(parent_key):
|
||||
return unset_source
|
||||
db_value: Final = db_values.get(field_name)
|
||||
if db_value is not None and not (isinstance(db_value, list) and len(db_value) == 0):
|
||||
return "db"
|
||||
return unset_source
|
||||
return "db" if field_name in db_values else unset_source
|
||||
|
||||
|
||||
@router.get(
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@ Tests for router settings management endpoints.
|
|||
Tests the GET endpoints for router settings and router fields.
|
||||
"""
|
||||
|
||||
from collections.abc import Mapping
|
||||
from typing import Any, Final
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
|
@ -22,15 +24,14 @@ from litellm.router import Router
|
|||
client = TestClient(app)
|
||||
|
||||
|
||||
def _stub_proxy_config(router_settings, config_router_settings):
|
||||
class _StubProxyConfig:
|
||||
def __init__(self):
|
||||
self.router_settings = router_settings
|
||||
class _StubProxyConfig:
|
||||
def __init__(self, router_settings: SettingsStore, config_router_settings: Mapping[str, Any]) -> None:
|
||||
self.router_settings: Final = router_settings
|
||||
self._config_router_settings: Final = dict(config_router_settings)
|
||||
|
||||
async def get_config(self, config_file_path=None):
|
||||
return {"router_settings": dict(config_router_settings)}
|
||||
|
||||
return _StubProxyConfig()
|
||||
async def get_config(self, config_file_path: str | None = None) -> dict[str, Any]:
|
||||
del config_file_path
|
||||
return {"router_settings": dict(self._config_router_settings)}
|
||||
|
||||
|
||||
class TestRouterSettingsEndpoints:
|
||||
|
|
@ -95,7 +96,7 @@ class TestRouterSettingsEndpoints:
|
|||
monkeypatch.setattr(
|
||||
proxy_server,
|
||||
"proxy_config",
|
||||
_stub_proxy_config(
|
||||
_StubProxyConfig(
|
||||
store,
|
||||
{"routing_strategy": "simple-shuffle", "num_retries": 3},
|
||||
),
|
||||
|
|
@ -140,7 +141,7 @@ class TestRouterSettingsEndpoints:
|
|||
monkeypatch.setattr(
|
||||
proxy_server,
|
||||
"proxy_config",
|
||||
_stub_proxy_config(SettingsStore("router_settings"), {}),
|
||||
_StubProxyConfig(SettingsStore("router_settings"), {}),
|
||||
)
|
||||
|
||||
admin_user = UserAPIKeyAuth(
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ Pins (PR2):
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Callable
|
||||
from collections.abc import Callable, Mapping
|
||||
from contextlib import AbstractContextManager
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
|
|
@ -22,6 +22,7 @@ import litellm
|
|||
from litellm.proxy import proxy_server
|
||||
from litellm.proxy._types import LitellmUserRoles
|
||||
from litellm.proxy.config_resolvers.settings_rules import JsonValue
|
||||
from litellm.proxy.config_resolvers.settings_store import SettingsStore
|
||||
|
||||
from .conftest import normalize # type: ignore[import-not-found]
|
||||
|
||||
|
|
@ -183,9 +184,13 @@ def test_model_settings_method_not_allowed(client, auth_as):
|
|||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _alerting_client(monkeypatch, *, yaml_values, db_row, live_args):
|
||||
from litellm.proxy.config_resolvers import SettingsStore
|
||||
|
||||
def _alerting_client(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
*,
|
||||
yaml_values: Mapping[str, JsonValue],
|
||||
db_row: Mapping[str, JsonValue],
|
||||
live_args: Mapping[str, JsonValue],
|
||||
) -> "SettingsStore":
|
||||
pc = MagicMock()
|
||||
row = MagicMock()
|
||||
row.param_value = db_row
|
||||
|
|
@ -206,7 +211,11 @@ def _alerting_client(monkeypatch, *, yaml_values, db_row, live_args):
|
|||
return store
|
||||
|
||||
|
||||
def test_alerting_settings_reports_sources(client, auth_as, monkeypatch):
|
||||
def test_alerting_settings_reports_sources(
|
||||
client: TestClient,
|
||||
auth_as: Callable[..., AbstractContextManager[None]],
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
_alerting_client(
|
||||
monkeypatch,
|
||||
yaml_values={
|
||||
|
|
@ -230,11 +239,21 @@ def test_alerting_settings_reports_sources(client, auth_as, monkeypatch):
|
|||
assert by_name["budget_alert_ttl"]["source"] == "default"
|
||||
|
||||
|
||||
def test_alerting_settings_reports_db_source_when_the_file_omits_alerting_args(client, auth_as, monkeypatch):
|
||||
def test_alerting_settings_reports_db_source_when_the_file_omits_alerting_args(
|
||||
client: TestClient,
|
||||
auth_as: Callable[..., AbstractContextManager[None]],
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
store = _alerting_client(
|
||||
monkeypatch,
|
||||
yaml_values={"alerting": ["slack"]},
|
||||
db_row={"alerting_args": {"outage_alert_ttl": 4242, "region_outage_alert_ttl": []}},
|
||||
db_row={
|
||||
"alerting_args": {
|
||||
"outage_alert_ttl": 4242,
|
||||
"region_outage_alert_ttl": [],
|
||||
"report_check_interval": None,
|
||||
}
|
||||
},
|
||||
live_args={"outage_alert_ttl": 4242},
|
||||
)
|
||||
|
||||
|
|
@ -244,13 +263,18 @@ def test_alerting_settings_reports_db_source_when_the_file_omits_alerting_args(c
|
|||
assert response.status_code == 200
|
||||
by_name = {entry["field_name"]: entry for entry in response.json()}
|
||||
|
||||
assert store.owned_by_config("alerting_args") is False
|
||||
assert store.source("alerting_args") == "db"
|
||||
assert by_name["outage_alert_ttl"]["source"] == "db"
|
||||
assert by_name["region_outage_alert_ttl"]["source"] == "default"
|
||||
assert by_name["region_outage_alert_ttl"]["source"] == "db"
|
||||
assert by_name["report_check_interval"]["source"] == "db"
|
||||
assert by_name["budget_alert_ttl"]["source"] == "default"
|
||||
|
||||
|
||||
def test_alerting_settings_reports_config_source_when_db_disagrees(client, auth_as, monkeypatch):
|
||||
def test_alerting_settings_reports_config_source_when_db_disagrees(
|
||||
client: TestClient,
|
||||
auth_as: Callable[..., AbstractContextManager[None]],
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
from litellm.proxy.config_resolvers import SettingsStore
|
||||
|
||||
db_alerting_args = {"daily_report_frequency": 7}
|
||||
|
|
@ -289,7 +313,7 @@ def test_alerting_settings_handles_empty_db_args(
|
|||
auth_as: Callable[..., AbstractContextManager[None]],
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
db_alerting_args: JsonValue,
|
||||
):
|
||||
) -> None:
|
||||
from litellm.proxy.config_resolvers import SettingsStore
|
||||
|
||||
pc = MagicMock()
|
||||
|
|
@ -318,6 +342,19 @@ def test_alerting_settings_handles_empty_db_args(
|
|||
assert by_name["budget_alert_ttl"]["source"] == "default"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("field_default", "expected"),
|
||||
[(43200, "default"), (None, "unset")],
|
||||
)
|
||||
def test_nested_setting_source_without_a_config_or_db_value(field_default: JsonValue, expected: str) -> None:
|
||||
store = SettingsStore("general_settings")
|
||||
store.load_yaml({})
|
||||
|
||||
assert (
|
||||
proxy_server._nested_setting_source(store, {}, "alerting_args", "budget_alert_ttl", field_default) == expected
|
||||
)
|
||||
|
||||
|
||||
def test_alerting_settings_no_db_error(client, auth_as, no_prisma):
|
||||
"""Pins ``GET /alerting/settings`` (error: db not connected)."""
|
||||
with auth_as(LitellmUserRoles.PROXY_ADMIN):
|
||||
|
|
|
|||
|
|
@ -1342,7 +1342,7 @@ class TestProxySettingEndpoints:
|
|||
where={"id": "ui_settings"}
|
||||
)
|
||||
|
||||
def test_get_ui_settings_reports_sources(self, monkeypatch):
|
||||
def test_get_ui_settings_reports_sources(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
from litellm.proxy import proxy_server
|
||||
|
|
@ -3532,7 +3532,7 @@ class TestPtuCostAttributionUISetting:
|
|||
|
||||
def test_reported_config_when_secret_manager_enables_the_flag(
|
||||
self, mock_auth: None, monkeypatch: pytest.MonkeyPatch
|
||||
):
|
||||
) -> None:
|
||||
from litellm.proxy.spend_tracking.ptu_feature_flag import PTU_COST_ATTRIBUTION_ENV_VAR
|
||||
|
||||
monkeypatch.delenv(PTU_COST_ATTRIBUTION_ENV_VAR, raising=False)
|
||||
|
|
@ -3550,7 +3550,7 @@ class TestPtuCostAttributionUISetting:
|
|||
|
||||
def test_reported_config_when_secret_manager_disables_the_flag(
|
||||
self, mock_auth: None, monkeypatch: pytest.MonkeyPatch
|
||||
):
|
||||
) -> None:
|
||||
from litellm.proxy.spend_tracking.ptu_feature_flag import PTU_COST_ATTRIBUTION_ENV_VAR
|
||||
|
||||
monkeypatch.delenv(PTU_COST_ATTRIBUTION_ENV_VAR, raising=False)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue