mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
fix(proxy): delete callbacks configured under callbacks or failure_callback
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
860bc7811d
commit
f19f6a4841
5 changed files with 328 additions and 15 deletions
|
|
@ -18467,6 +18467,21 @@ async def delete_config_general_settings(
|
|||
return response
|
||||
|
||||
|
||||
_CALLBACK_LIST_KEYS: Final = ("success_callback", "failure_callback", "callbacks")
|
||||
|
||||
|
||||
def _configured_callback_names(value: object) -> tuple[object, ...]:
|
||||
if isinstance(value, str):
|
||||
return (value,)
|
||||
if isinstance(value, (list, tuple, dict)):
|
||||
return tuple(value)
|
||||
return ()
|
||||
|
||||
|
||||
def _is_callback_name(entry: object, callback_name: str) -> bool:
|
||||
return isinstance(entry, str) and entry.lower() == callback_name
|
||||
|
||||
|
||||
@router.post(
|
||||
"/config/callback/delete",
|
||||
tags=["config.yaml"],
|
||||
|
|
@ -18507,19 +18522,27 @@ async def delete_callback(
|
|||
|
||||
# Check if callback exists in current configuration
|
||||
litellm_settings: Final = config.get("litellm_settings", {})
|
||||
success_callbacks: Final = litellm_settings.get("success_callback", [])
|
||||
configured_lists: Final = {
|
||||
key: _configured_callback_names(litellm_settings.get(key)) for key in _CALLBACK_LIST_KEYS
|
||||
}
|
||||
matching_keys: Final = tuple(
|
||||
key
|
||||
for key, names in configured_lists.items()
|
||||
if any(_is_callback_name(entry, callback_name) for entry in names)
|
||||
)
|
||||
|
||||
if callback_name not in success_callbacks:
|
||||
if not matching_keys:
|
||||
raise HTTPException(
|
||||
status_code=404,
|
||||
detail={"error": f"Callback '{callback_name}' not found in active configuration"},
|
||||
)
|
||||
|
||||
before_success_callbacks: Final = list(success_callbacks)
|
||||
|
||||
# Remove callback from success_callback list
|
||||
success_callbacks.remove(callback_name)
|
||||
config.setdefault("litellm_settings", {})["success_callback"] = success_callbacks
|
||||
before_callbacks: Final = {key: list(configured_lists[key]) for key in matching_keys}
|
||||
after_callbacks: Final = {
|
||||
key: [entry for entry in configured_lists[key] if not _is_callback_name(entry, callback_name)]
|
||||
for key in matching_keys
|
||||
}
|
||||
config["litellm_settings"] = {**litellm_settings, **after_callbacks}
|
||||
|
||||
# Save the updated configuration
|
||||
await proxy_config.save_config(new_config=config)
|
||||
|
|
@ -18528,8 +18551,8 @@ async def delete_callback(
|
|||
create_config_audit_log(
|
||||
"litellm_settings",
|
||||
"deleted",
|
||||
{"success_callback": before_success_callbacks},
|
||||
{"success_callback": success_callbacks},
|
||||
before_callbacks,
|
||||
after_callbacks,
|
||||
user_api_key_dict,
|
||||
)
|
||||
)
|
||||
|
|
@ -18537,10 +18560,13 @@ async def delete_callback(
|
|||
# Restart the proxy to apply changes
|
||||
await proxy_config.add_deployment(prisma_client=prisma_client, proxy_logging_obj=proxy_logging_obj)
|
||||
|
||||
updated_settings: Final = config["litellm_settings"]
|
||||
return {
|
||||
"message": f"Successfully deleted callback: {callback_name}",
|
||||
"removed_callback": callback_name,
|
||||
"remaining_callbacks": success_callbacks,
|
||||
"remaining_callbacks": [
|
||||
entry for key in _CALLBACK_LIST_KEYS for entry in _configured_callback_names(updated_settings.get(key))
|
||||
],
|
||||
"deleted_at": datetime.now().isoformat(),
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -17,35 +17,40 @@ import base64
|
|||
import json
|
||||
import os
|
||||
import time
|
||||
from collections.abc import Callable
|
||||
from dataclasses import dataclass
|
||||
from typing import Callable, Literal
|
||||
from typing import Literal
|
||||
|
||||
import pytest
|
||||
from pydantic import BaseModel, ConfigDict, Field, JsonValue, TypeAdapter, ValidationError
|
||||
|
||||
from e2e_config import POLL_INTERVAL, POLL_TIMEOUT, settle_propagation
|
||||
from proxy_client import ProxyClient
|
||||
from e2e_http import (
|
||||
URL,
|
||||
AuthHeaders,
|
||||
require_successful_call,
|
||||
NoBody,
|
||||
Result,
|
||||
StreamingResponse,
|
||||
Success,
|
||||
get,
|
||||
require_successful_call,
|
||||
unwrap,
|
||||
)
|
||||
from models import (
|
||||
AnthropicMessagesBody,
|
||||
CallbackDeleteBody,
|
||||
CallbackDeleteResponse,
|
||||
ChatBody,
|
||||
ChatMessage,
|
||||
ChatResponse,
|
||||
ChatTool,
|
||||
ChatToolFunction,
|
||||
ConfigCallbacksResponse,
|
||||
ConfigUpdateCallbacksBody,
|
||||
ConfigUpdateResponse,
|
||||
KeyGenerateBody,
|
||||
KeyLoggingCallback,
|
||||
KeyLoggingCallbackVars,
|
||||
KeyMetadata,
|
||||
LitellmCallbackSettings,
|
||||
LiteLLMParamsBody,
|
||||
OrgDeleteBody,
|
||||
OrgNewBody,
|
||||
|
|
@ -58,6 +63,8 @@ from models import (
|
|||
UserNewBody,
|
||||
UserNewResponse,
|
||||
)
|
||||
from proxy_client import ProxyClient
|
||||
from pydantic import BaseModel, ConfigDict, Field, JsonValue, TypeAdapter, ValidationError
|
||||
|
||||
# Deliberately invalid *upstream provider* key for failure-path tests.
|
||||
# Not a LiteLLM virtual key; OpenAI must reject it after the proxy accepts the call.
|
||||
|
|
@ -488,6 +495,56 @@ class LoggingClient:
|
|||
def delete_model(self, model_id: str) -> None:
|
||||
self.proxy.delete_model(model_id)
|
||||
|
||||
# ---- global callback configuration (Admin UI Logging & Alerts surface) ----
|
||||
|
||||
def set_litellm_callbacks(self, settings: LitellmCallbackSettings) -> ConfigUpdateResponse:
|
||||
"""POST /config/update, the route the Admin UI's Logging page uses to add
|
||||
a callback."""
|
||||
return unwrap(
|
||||
self.proxy.transport.post(
|
||||
"/config/update",
|
||||
headers=self.proxy.transport.master,
|
||||
json=ConfigUpdateCallbacksBody(litellm_settings=settings),
|
||||
response_type=ConfigUpdateResponse,
|
||||
)
|
||||
)
|
||||
|
||||
def clear_litellm_callbacks(self, *keys: Literal["callbacks", "failure_callback"]) -> None:
|
||||
"""Empty the given litellm_settings callback lists so teardown leaves the
|
||||
stored row as it was. success_callback is absent on purpose: /config/update
|
||||
unions that key instead of replacing it, so entries there can only leave
|
||||
through /config/callback/delete."""
|
||||
_ = self.set_litellm_callbacks(
|
||||
LitellmCallbackSettings(
|
||||
callbacks=[] if "callbacks" in keys else None,
|
||||
failure_callback=[] if "failure_callback" in keys else None,
|
||||
)
|
||||
)
|
||||
|
||||
def config_callback_names(self) -> set[str]:
|
||||
"""Every name GET /get/config/callbacks lists as an active callback."""
|
||||
return {
|
||||
entry.name
|
||||
for entry in unwrap(
|
||||
self.proxy.transport.get(
|
||||
"/get/config/callbacks",
|
||||
headers=self.proxy.transport.master,
|
||||
params=NoBody(),
|
||||
response_type=ConfigCallbacksResponse,
|
||||
)
|
||||
).callbacks
|
||||
}
|
||||
|
||||
def delete_config_callback(self, callback_name: str) -> Result[CallbackDeleteResponse]:
|
||||
"""POST /config/callback/delete. Returns the raw Result so the caller can
|
||||
unwrap for the success contract or ignore it for best-effort cleanup."""
|
||||
return self.proxy.transport.post(
|
||||
"/config/callback/delete",
|
||||
headers=self.proxy.transport.master,
|
||||
json=CallbackDeleteBody(callback_name=callback_name),
|
||||
response_type=CallbackDeleteResponse,
|
||||
)
|
||||
|
||||
def chat(self, key: str, model: str, text: str) -> ChatResponse:
|
||||
return unwrap(
|
||||
self.proxy.chat(
|
||||
|
|
|
|||
85
tests/e2e/logging/test_callback_delete_e2e.py
Normal file
85
tests/e2e/logging/test_callback_delete_e2e.py
Normal file
|
|
@ -0,0 +1,85 @@
|
|||
"""Live e2e: a callback the Admin UI lists must also be deletable.
|
||||
|
||||
GET /get/config/callbacks enumerates litellm_settings.success_callback,
|
||||
failure_callback and callbacks, plus runtime-only loggers, but
|
||||
POST /config/callback/delete only looks inside success_callback. A callback
|
||||
configured under `callbacks` or `failure_callback` is listed as active on the
|
||||
Logging & Alerts -> Logging Callbacks page yet its delete answers 404 "Callback
|
||||
not found in active configuration". The success_callback case is the control:
|
||||
it proves the add/list/delete round trip works when the configured key is the
|
||||
one the delete route actually reads.
|
||||
"""
|
||||
|
||||
from collections.abc import Callable
|
||||
|
||||
import pytest
|
||||
from e2e_http import unwrap
|
||||
from lifecycle import ResourceManager
|
||||
from logging_client import LoggingClient
|
||||
from models import LitellmCallbackSettings
|
||||
|
||||
pytestmark = pytest.mark.e2e
|
||||
|
||||
CALLBACK_NAME = "datadog"
|
||||
|
||||
|
||||
def _assert_listed_then_deleted(
|
||||
client: LoggingClient,
|
||||
resources: ResourceManager,
|
||||
settings: LitellmCallbackSettings,
|
||||
cleanup: Callable[[], object],
|
||||
) -> None:
|
||||
response = client.set_litellm_callbacks(settings)
|
||||
assert "success" in response.message.lower(), (
|
||||
f"/config/update reported {response.message!r}, expected a success message"
|
||||
)
|
||||
resources.defer(cleanup)
|
||||
|
||||
names = client.config_callback_names()
|
||||
assert CALLBACK_NAME in names, (
|
||||
f"GET /get/config/callbacks does not list {CALLBACK_NAME!r} after it was "
|
||||
f"configured via /config/update; listed {sorted(names)}"
|
||||
)
|
||||
|
||||
deleted = unwrap(client.delete_config_callback(CALLBACK_NAME))
|
||||
assert deleted.removed_callback == CALLBACK_NAME, (
|
||||
f"/config/callback/delete removed {deleted.removed_callback!r}, expected {CALLBACK_NAME!r}"
|
||||
)
|
||||
|
||||
names_after = client.config_callback_names()
|
||||
assert CALLBACK_NAME not in names_after, (
|
||||
f"GET /get/config/callbacks still lists {CALLBACK_NAME!r} after a successful "
|
||||
f"delete; listed {sorted(names_after)}"
|
||||
)
|
||||
|
||||
|
||||
class TestCallbackDelete:
|
||||
def test_callback_under_callbacks_key_is_listed_and_deletable(
|
||||
self, client: LoggingClient, resources: ResourceManager
|
||||
) -> None:
|
||||
_assert_listed_then_deleted(
|
||||
client,
|
||||
resources,
|
||||
LitellmCallbackSettings(callbacks=[CALLBACK_NAME]),
|
||||
lambda: client.clear_litellm_callbacks("callbacks"),
|
||||
)
|
||||
|
||||
def test_callback_under_failure_callback_key_is_listed_and_deletable(
|
||||
self, client: LoggingClient, resources: ResourceManager
|
||||
) -> None:
|
||||
_assert_listed_then_deleted(
|
||||
client,
|
||||
resources,
|
||||
LitellmCallbackSettings(failure_callback=[CALLBACK_NAME]),
|
||||
lambda: client.clear_litellm_callbacks("failure_callback"),
|
||||
)
|
||||
|
||||
def test_callback_under_success_callback_key_is_deletable(
|
||||
self, client: LoggingClient, resources: ResourceManager
|
||||
) -> None:
|
||||
_assert_listed_then_deleted(
|
||||
client,
|
||||
resources,
|
||||
LitellmCallbackSettings(success_callback=[CALLBACK_NAME]),
|
||||
lambda: client.delete_config_callback(CALLBACK_NAME),
|
||||
)
|
||||
|
|
@ -1147,6 +1147,55 @@ class ConfigFieldList(RootModel[tuple[ConfigField, ...]]):
|
|||
"""GET /config/list answers with a bare array of general_settings fields."""
|
||||
|
||||
|
||||
class LitellmCallbackSettings(BaseModel):
|
||||
"""The litellm_settings callback lists /config/update merges. Unset keys are
|
||||
left out of the request so the proxy keeps whatever the stored row holds."""
|
||||
|
||||
model_config = ConfigDict(extra="ignore")
|
||||
success_callback: list[str] | None = None
|
||||
failure_callback: list[str] | None = None
|
||||
callbacks: list[str] | None = None
|
||||
|
||||
|
||||
class ConfigUpdateCallbacksBody(BaseModel):
|
||||
"""POST /config/update body: the same shape the Admin UI's Logging page
|
||||
sends when an admin adds a callback (litellm_settings plus, in the UI's
|
||||
case, environment_variables the test does not need)."""
|
||||
|
||||
litellm_settings: LitellmCallbackSettings
|
||||
|
||||
|
||||
class ConfigUpdateResponse(BaseModel):
|
||||
model_config = ConfigDict(extra="ignore")
|
||||
message: str
|
||||
|
||||
|
||||
class ConfigCallbackEntry(BaseModel):
|
||||
"""One row of GET /get/config/callbacks `callbacks`. `type` is which
|
||||
configured list the entry came from (success, failure, success_and_failure)."""
|
||||
|
||||
model_config = ConfigDict(extra="ignore")
|
||||
name: str
|
||||
type: str | None = None
|
||||
read_only: bool | None = None
|
||||
|
||||
|
||||
class ConfigCallbacksResponse(BaseModel):
|
||||
model_config = ConfigDict(extra="ignore")
|
||||
callbacks: list[ConfigCallbackEntry] = []
|
||||
|
||||
|
||||
class CallbackDeleteBody(BaseModel):
|
||||
callback_name: str
|
||||
|
||||
|
||||
class CallbackDeleteResponse(BaseModel):
|
||||
model_config = ConfigDict(extra="ignore")
|
||||
message: str
|
||||
removed_callback: str
|
||||
remaining_callbacks: list[str] = []
|
||||
|
||||
|
||||
class CostMapEntry(BaseModel):
|
||||
model_config = ConfigDict(extra="ignore")
|
||||
litellm_provider: str | None = None
|
||||
|
|
|
|||
|
|
@ -944,6 +944,102 @@ def test_config_callback_delete_not_found(client, auth_as, mock_prisma, monkeypa
|
|||
assert "langfuse" in str(response.json()).lower() or "not found" in str(response.json()).lower()
|
||||
|
||||
|
||||
def _delete_callback_roundtrip(client, auth_as, mock_prisma, monkeypatch, litellm_settings, callback_name):
|
||||
"""POST /config/callback/delete as admin against a stubbed config, and
|
||||
return (response, saved litellm_settings) so each case pins the full body
|
||||
plus the exact config persisted."""
|
||||
from litellm.proxy import proxy_server as ps
|
||||
from litellm.proxy._types import LitellmUserRoles
|
||||
|
||||
_install_litellm_config(mock_prisma)
|
||||
monkeypatch.setattr(ps, "prisma_client", mock_prisma)
|
||||
monkeypatch.setattr(ps, "store_model_in_db", True)
|
||||
|
||||
fake_proxy_config = MagicMock()
|
||||
fake_proxy_config.get_config = AsyncMock(return_value={"litellm_settings": litellm_settings})
|
||||
fake_proxy_config.save_config = AsyncMock()
|
||||
fake_proxy_config.add_deployment = AsyncMock()
|
||||
monkeypatch.setattr(ps, "proxy_config", fake_proxy_config)
|
||||
|
||||
with auth_as(LitellmUserRoles.PROXY_ADMIN):
|
||||
response = client.post("/config/callback/delete", json={"callback_name": callback_name})
|
||||
saved = fake_proxy_config.save_config.await_args.kwargs["new_config"]["litellm_settings"]
|
||||
return response, saved
|
||||
|
||||
|
||||
def test_config_callback_delete_from_failure_callback(client, auth_as, mock_prisma, monkeypatch):
|
||||
"""A callback configured only under litellm_settings.failure_callback is
|
||||
listed by GET /get/config/callbacks, so the delete route must remove it."""
|
||||
response, saved = _delete_callback_roundtrip(
|
||||
client,
|
||||
auth_as,
|
||||
mock_prisma,
|
||||
monkeypatch,
|
||||
{"success_callback": ["slack"], "failure_callback": ["datadog", "sentry"]},
|
||||
"datadog",
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert response.json()["removed_callback"] == "datadog"
|
||||
assert saved == {"success_callback": ["slack"], "failure_callback": ["sentry"]}
|
||||
assert response.json()["remaining_callbacks"] == ["slack", "sentry"]
|
||||
|
||||
|
||||
def test_config_callback_delete_from_callbacks_list(client, auth_as, mock_prisma, monkeypatch):
|
||||
"""A callback configured only under litellm_settings.callbacks is deleted
|
||||
while unrelated lists pass through untouched."""
|
||||
response, saved = _delete_callback_roundtrip(
|
||||
client,
|
||||
auth_as,
|
||||
mock_prisma,
|
||||
monkeypatch,
|
||||
{"success_callback": ["langfuse"], "callbacks": ["datadog"], "drop_params": True},
|
||||
"datadog",
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert response.json()["removed_callback"] == "datadog"
|
||||
assert saved == {"success_callback": ["langfuse"], "callbacks": [], "drop_params": True}
|
||||
assert response.json()["remaining_callbacks"] == ["langfuse"]
|
||||
|
||||
|
||||
def test_config_callback_delete_from_multiple_lists(client, auth_as, mock_prisma, monkeypatch):
|
||||
"""A name present in both success_callback and callbacks is removed from
|
||||
both, and remaining_callbacks reports the leftovers of every list."""
|
||||
response, saved = _delete_callback_roundtrip(
|
||||
client,
|
||||
auth_as,
|
||||
mock_prisma,
|
||||
monkeypatch,
|
||||
{
|
||||
"success_callback": ["datadog", "slack"],
|
||||
"failure_callback": ["sentry"],
|
||||
"callbacks": ["langfuse", "datadog"],
|
||||
},
|
||||
"datadog",
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert saved == {
|
||||
"success_callback": ["slack"],
|
||||
"failure_callback": ["sentry"],
|
||||
"callbacks": ["langfuse"],
|
||||
}
|
||||
assert response.json()["remaining_callbacks"] == ["slack", "sentry", "langfuse"]
|
||||
|
||||
|
||||
def test_config_callback_delete_case_insensitive(client, auth_as, mock_prisma, monkeypatch):
|
||||
"""A mixed-case request name still matches the configured lowercase entry."""
|
||||
response, saved = _delete_callback_roundtrip(
|
||||
client,
|
||||
auth_as,
|
||||
mock_prisma,
|
||||
monkeypatch,
|
||||
{"callbacks": ["datadog", "otel"]},
|
||||
"DataDog",
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert response.json()["removed_callback"] == "datadog"
|
||||
assert saved == {"callbacks": ["otel"]}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# GET /get/config/callbacks
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue