[Fix] UI - Delete Callbacks Failing (#16473)
* Temp commit for branch switching * Created normalize callback name util function and tests
This commit is contained in:
parent
92bd12c862
commit
cb27d6c456
@ -1,4 +1,4 @@
|
||||
from typing import Any, Dict, List, Literal, Optional
|
||||
from typing import Any, Dict, List, Literal, Optional, Iterable
|
||||
|
||||
import litellm
|
||||
from litellm import get_secret
|
||||
@ -382,3 +382,8 @@ def get_metadata_variable_name_from_kwargs(
|
||||
- LiteLLM is now moving to using `litellm_metadata` for our metadata
|
||||
"""
|
||||
return "litellm_metadata" if "litellm_metadata" in kwargs else "metadata"
|
||||
|
||||
def normalize_callback_names(callbacks: Iterable[Any]) -> List[Any]:
|
||||
if callbacks is None:
|
||||
return []
|
||||
return [c.lower() if isinstance(c, str) else c for c in callbacks]
|
||||
@ -48,6 +48,8 @@ from litellm.types.utils import (
|
||||
)
|
||||
from litellm.utils import load_credentials_from_list
|
||||
|
||||
from litellm.proxy.common_utils.callback_utils import normalize_callback_names
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from aiohttp import ClientSession
|
||||
from opentelemetry.trace import Span as _Span
|
||||
@ -9052,9 +9054,10 @@ async def update_config(config_info: ConfigYAML): # noqa: PLR0915
|
||||
if isinstance(
|
||||
config["litellm_settings"]["success_callback"], list
|
||||
) and isinstance(updated_litellm_settings["success_callback"], list):
|
||||
updated_success_callbacks_normalized = normalize_callback_names(updated_litellm_settings["success_callback"])
|
||||
combined_success_callback = (
|
||||
config["litellm_settings"]["success_callback"]
|
||||
+ updated_litellm_settings["success_callback"]
|
||||
+ updated_success_callbacks_normalized
|
||||
)
|
||||
combined_success_callback = list(set(combined_success_callback))
|
||||
config["litellm_settings"][
|
||||
|
||||
@ -2401,3 +2401,61 @@ def test_non_root_ui_path_logic(monkeypatch, tmp_path, ui_exists, ui_has_content
|
||||
error_calls = [call[0][0] for call in mock_logger.error.call_args_list]
|
||||
assert any("Path exists:" in call for call in error_calls)
|
||||
assert mock_logger.info.call_count == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_config_success_callback_normalization():
|
||||
"""
|
||||
Ensure success_callback values are normalized to lowercase when updating config.
|
||||
This prevents delete_callback (which searches lowercase) from failing on mixed case inputs like 'SQS'.
|
||||
"""
|
||||
import litellm.proxy.proxy_server as proxy_server
|
||||
from litellm.proxy._types import ConfigYAML
|
||||
|
||||
# Ensure feature is enabled and prisma_client is set
|
||||
setattr(proxy_server, "store_model_in_db", True)
|
||||
setattr(proxy_server, "proxy_logging_obj", MagicMock())
|
||||
|
||||
class MockPrisma:
|
||||
def __init__(self):
|
||||
self.db = MagicMock()
|
||||
self.db.litellm_config = MagicMock()
|
||||
self.db.litellm_config.upsert = AsyncMock()
|
||||
|
||||
# proxy_server.update_config expects this to be sync returning a dict
|
||||
def jsonify_object(self, obj):
|
||||
return obj
|
||||
|
||||
setattr(proxy_server, "prisma_client", MockPrisma())
|
||||
|
||||
class MockProxyConfig:
|
||||
def __init__(self):
|
||||
self.saved_config = None
|
||||
|
||||
async def get_config(self):
|
||||
# Existing config has one lowercase callback already
|
||||
return {"litellm_settings": {"success_callback": ["langfuse"]}}
|
||||
|
||||
async def save_config(self, new_config: dict):
|
||||
self.saved_config = new_config
|
||||
|
||||
async def add_deployment(self, prisma_client=None, proxy_logging_obj=None):
|
||||
return None
|
||||
|
||||
mock_proxy_config = MockProxyConfig()
|
||||
setattr(proxy_server, "proxy_config", mock_proxy_config)
|
||||
|
||||
# Update config with mixed-case callbacks - expect normalization to lowercase
|
||||
config_update = ConfigYAML(litellm_settings={"success_callback": ["SQS", "sQs"]})
|
||||
await proxy_server.update_config(config_update)
|
||||
|
||||
saved = mock_proxy_config.saved_config
|
||||
assert saved is not None, "save_config was not called"
|
||||
callbacks = saved["litellm_settings"]["success_callback"]
|
||||
|
||||
# Deduped and normalized
|
||||
assert "sqs" in callbacks
|
||||
assert "SQS" not in callbacks
|
||||
assert "sQs" not in callbacks
|
||||
# Existing callback should still be present
|
||||
assert "langfuse" in callbacks
|
||||
|
||||
@ -7,6 +7,7 @@ sys.path.insert(
|
||||
|
||||
from litellm.proxy.common_utils.callback_utils import (
|
||||
get_remaining_tokens_and_requests_from_request_data,
|
||||
normalize_callback_names,
|
||||
)
|
||||
|
||||
|
||||
@ -27,3 +28,13 @@ def test_get_remaining_tokens_and_requests_from_request_data():
|
||||
f"x-litellm-key-remaining-requests-{expected_name}": 100,
|
||||
f"x-litellm-key-remaining-tokens-{expected_name}": 200,
|
||||
}
|
||||
|
||||
|
||||
def test_normalize_callback_names_none_returns_empty_list():
|
||||
assert normalize_callback_names(None) == []
|
||||
assert normalize_callback_names([]) == []
|
||||
|
||||
|
||||
def test_normalize_callback_names_lowercases_strings():
|
||||
assert normalize_callback_names(["SQS", "S3", "CUSTOM_CALLBACK"]) == ["sqs", "s3", "custom_callback"]
|
||||
|
||||
|
||||
@ -7,7 +7,7 @@ interface CallbackConfig {
|
||||
description: string;
|
||||
}
|
||||
|
||||
const asset_logos_folder = '/ui/assets/logos/';
|
||||
const asset_logos_folder = "/ui/assets/logos/";
|
||||
|
||||
export const CALLBACK_CONFIGS: CallbackConfig[] = [
|
||||
{
|
||||
@ -16,10 +16,10 @@ export const CALLBACK_CONFIGS: CallbackConfig[] = [
|
||||
logo: `${asset_logos_folder}arize.png`,
|
||||
supports_key_team_logging: true,
|
||||
dynamic_params: {
|
||||
"arize_api_key": "password",
|
||||
"arize_space_key": "password",
|
||||
arize_api_key: "password",
|
||||
arize_space_key: "password",
|
||||
},
|
||||
description: "Arize Logging Integration"
|
||||
description: "Arize Logging Integration",
|
||||
},
|
||||
{
|
||||
id: "braintrust",
|
||||
@ -27,10 +27,10 @@ export const CALLBACK_CONFIGS: CallbackConfig[] = [
|
||||
logo: `${asset_logos_folder}braintrust.png`,
|
||||
supports_key_team_logging: false,
|
||||
dynamic_params: {
|
||||
"braintrust_api_key": "password",
|
||||
"braintrust_project_name": "text"
|
||||
braintrust_api_key: "password",
|
||||
braintrust_project_name: "text",
|
||||
},
|
||||
description: "Braintrust Logging Integration"
|
||||
description: "Braintrust Logging Integration",
|
||||
},
|
||||
{
|
||||
id: "custom_callback_api",
|
||||
@ -38,10 +38,10 @@ export const CALLBACK_CONFIGS: CallbackConfig[] = [
|
||||
logo: `${asset_logos_folder}custom.svg`,
|
||||
supports_key_team_logging: true,
|
||||
dynamic_params: {
|
||||
"custom_callback_api_url": "text",
|
||||
"custom_callback_api_headers": "text"
|
||||
custom_callback_api_url: "text",
|
||||
custom_callback_api_headers: "text",
|
||||
},
|
||||
description: "Custom Callback API Logging Integration"
|
||||
description: "Custom Callback API Logging Integration",
|
||||
},
|
||||
{
|
||||
id: "datadog",
|
||||
@ -49,10 +49,10 @@ export const CALLBACK_CONFIGS: CallbackConfig[] = [
|
||||
logo: `${asset_logos_folder}datadog.png`,
|
||||
supports_key_team_logging: false,
|
||||
dynamic_params: {
|
||||
"dd_api_key": "password",
|
||||
"dd_site": "text"
|
||||
dd_api_key: "password",
|
||||
dd_site: "text",
|
||||
},
|
||||
description: "Datadog Logging Integration"
|
||||
description: "Datadog Logging Integration",
|
||||
},
|
||||
{
|
||||
id: "lago",
|
||||
@ -60,10 +60,10 @@ export const CALLBACK_CONFIGS: CallbackConfig[] = [
|
||||
logo: `${asset_logos_folder}lago.svg`,
|
||||
supports_key_team_logging: false,
|
||||
dynamic_params: {
|
||||
"lago_api_url": "text",
|
||||
"lago_api_key": "password"
|
||||
lago_api_url: "text",
|
||||
lago_api_key: "password",
|
||||
},
|
||||
description: "Lago Billing Logging Integration"
|
||||
description: "Lago Billing Logging Integration",
|
||||
},
|
||||
{
|
||||
id: "langfuse",
|
||||
@ -71,11 +71,11 @@ export const CALLBACK_CONFIGS: CallbackConfig[] = [
|
||||
logo: `${asset_logos_folder}langfuse.png`,
|
||||
supports_key_team_logging: true,
|
||||
dynamic_params: {
|
||||
"langfuse_public_key": "text",
|
||||
"langfuse_secret_key": "password",
|
||||
"langfuse_host": "text"
|
||||
langfuse_public_key: "text",
|
||||
langfuse_secret_key: "password",
|
||||
langfuse_host: "text",
|
||||
},
|
||||
description: "Langfuse v2 Logging Integration"
|
||||
description: "Langfuse v2 Logging Integration",
|
||||
},
|
||||
{
|
||||
id: "langfuse_otel",
|
||||
@ -83,11 +83,11 @@ export const CALLBACK_CONFIGS: CallbackConfig[] = [
|
||||
logo: `${asset_logos_folder}langfuse.png`,
|
||||
supports_key_team_logging: true,
|
||||
dynamic_params: {
|
||||
"langfuse_public_key": "text",
|
||||
"langfuse_secret_key": "password",
|
||||
"langfuse_host": "text"
|
||||
langfuse_public_key: "text",
|
||||
langfuse_secret_key: "password",
|
||||
langfuse_host: "text",
|
||||
},
|
||||
description: "Langfuse v3 OTEL Logging Integration"
|
||||
description: "Langfuse v3 OTEL Logging Integration",
|
||||
},
|
||||
{
|
||||
id: "langsmith",
|
||||
@ -95,12 +95,12 @@ export const CALLBACK_CONFIGS: CallbackConfig[] = [
|
||||
logo: `${asset_logos_folder}langsmith.png`,
|
||||
supports_key_team_logging: true,
|
||||
dynamic_params: {
|
||||
"langsmith_api_key": "password",
|
||||
"langsmith_project": "text",
|
||||
"langsmith_base_url": "text",
|
||||
"langsmith_sampling_rate": "number"
|
||||
langsmith_api_key: "password",
|
||||
langsmith_project: "text",
|
||||
langsmith_base_url: "text",
|
||||
langsmith_sampling_rate: "number",
|
||||
},
|
||||
description: "Langsmith Logging Integration"
|
||||
description: "Langsmith Logging Integration",
|
||||
},
|
||||
{
|
||||
id: "openmeter",
|
||||
@ -108,10 +108,10 @@ export const CALLBACK_CONFIGS: CallbackConfig[] = [
|
||||
logo: `${asset_logos_folder}openmeter.png`,
|
||||
supports_key_team_logging: false,
|
||||
dynamic_params: {
|
||||
"openmeter_api_key": "password",
|
||||
"openmeter_base_url": "text"
|
||||
openmeter_api_key: "password",
|
||||
openmeter_base_url: "text",
|
||||
},
|
||||
description: "OpenMeter Logging Integration"
|
||||
description: "OpenMeter Logging Integration",
|
||||
},
|
||||
{
|
||||
id: "otel",
|
||||
@ -119,10 +119,10 @@ export const CALLBACK_CONFIGS: CallbackConfig[] = [
|
||||
logo: `${asset_logos_folder}otel.png`,
|
||||
supports_key_team_logging: false,
|
||||
dynamic_params: {
|
||||
"otel_endpoint": "text",
|
||||
"otel_headers": "text"
|
||||
otel_endpoint: "text",
|
||||
otel_headers: "text",
|
||||
},
|
||||
description: "OpenTelemetry Logging Integration"
|
||||
description: "OpenTelemetry Logging Integration",
|
||||
},
|
||||
{
|
||||
id: "s3",
|
||||
@ -130,48 +130,70 @@ export const CALLBACK_CONFIGS: CallbackConfig[] = [
|
||||
logo: `${asset_logos_folder}aws.svg`,
|
||||
supports_key_team_logging: false,
|
||||
dynamic_params: {
|
||||
"s3_bucket_name": "text",
|
||||
"aws_access_key_id": "password",
|
||||
"aws_secret_access_key": "password",
|
||||
"aws_region": "text"
|
||||
s3_bucket_name: "text",
|
||||
aws_access_key_id: "password",
|
||||
aws_secret_access_key: "password",
|
||||
aws_region: "text",
|
||||
},
|
||||
description: "S3 Bucket (AWS) Logging Integration"
|
||||
}
|
||||
description: "S3 Bucket (AWS) Logging Integration",
|
||||
},
|
||||
{
|
||||
id: "SQS",
|
||||
displayName: "SQS",
|
||||
logo: `${asset_logos_folder}aws.svg`,
|
||||
supports_key_team_logging: false,
|
||||
dynamic_params: {
|
||||
sqs_queue_url: "text",
|
||||
aws_access_key_id: "password",
|
||||
aws_secret_access_key: "password",
|
||||
aws_region: "text",
|
||||
},
|
||||
description: "SQS Queue (AWS) Logging Integration",
|
||||
},
|
||||
];
|
||||
|
||||
// Create callbackInfo object mapping display names to config objects
|
||||
export const callbackInfo: Record<string, CallbackConfig> = CALLBACK_CONFIGS.reduce((acc, config) => {
|
||||
acc[config.displayName] = config;
|
||||
return acc;
|
||||
}, {} as Record<string, CallbackConfig>);
|
||||
export const callbackInfo: Record<string, CallbackConfig> = CALLBACK_CONFIGS.reduce(
|
||||
(acc, config) => {
|
||||
acc[config.displayName] = config;
|
||||
return acc;
|
||||
},
|
||||
{} as Record<string, CallbackConfig>,
|
||||
);
|
||||
|
||||
// Create callback_map mapping display names to internal IDs
|
||||
export const callback_map: Record<string, string> = CALLBACK_CONFIGS.reduce((acc, config) => {
|
||||
acc[config.displayName] = config.id;
|
||||
return acc;
|
||||
}, {} as Record<string, string>);
|
||||
export const callback_map: Record<string, string> = CALLBACK_CONFIGS.reduce(
|
||||
(acc, config) => {
|
||||
acc[config.displayName] = config.id;
|
||||
return acc;
|
||||
},
|
||||
{} as Record<string, string>,
|
||||
);
|
||||
|
||||
// create reverse_callback_map to map internal IDs to display names
|
||||
export const reverse_callback_map: Record<string, string> = CALLBACK_CONFIGS.reduce((acc, config) => {
|
||||
acc[config.id] = config.displayName;
|
||||
return acc;
|
||||
}, {} as Record<string, string>);
|
||||
export const reverse_callback_map: Record<string, string> = CALLBACK_CONFIGS.reduce(
|
||||
(acc, config) => {
|
||||
acc[config.id] = config.displayName;
|
||||
return acc;
|
||||
},
|
||||
{} as Record<string, string>,
|
||||
);
|
||||
|
||||
// Function to map display names to internal names
|
||||
export const mapDisplayToInternalNames = (displayNames: string[]): string[] => {
|
||||
return displayNames.map(name => callback_map[name] || name);
|
||||
return displayNames.map((name) => callback_map[name] || name);
|
||||
};
|
||||
|
||||
// Function to map internal names to display names
|
||||
export const mapInternalToDisplayNames = (internalNames: string[]): string[] => {
|
||||
return internalNames.map(name => reverse_callback_map[name] || name);
|
||||
return internalNames.map((name) => reverse_callback_map[name] || name);
|
||||
};
|
||||
|
||||
// Utility functions for easy access
|
||||
export const getCallbackById = (id: string): CallbackConfig | undefined => {
|
||||
return CALLBACK_CONFIGS.find(callback => callback.id === id);
|
||||
return CALLBACK_CONFIGS.find((callback) => callback.id === id);
|
||||
};
|
||||
|
||||
export const getCallbackByDisplayName = (displayName: string): CallbackConfig | undefined => {
|
||||
return CALLBACK_CONFIGS.find(callback => callback.displayName === displayName);
|
||||
return CALLBACK_CONFIGS.find((callback) => callback.displayName === displayName);
|
||||
};
|
||||
|
||||
@ -565,7 +565,7 @@ const Settings: React.FC<SettingsPageProps> = ({ accessToken, userRole, userID,
|
||||
|
||||
<Modal
|
||||
title="Add Logging Callback"
|
||||
visible={showAddCallbacksModal}
|
||||
open={showAddCallbacksModal}
|
||||
width={800}
|
||||
onCancel={() => {
|
||||
setShowAddCallbacksModal(false);
|
||||
@ -682,7 +682,7 @@ const Settings: React.FC<SettingsPageProps> = ({ accessToken, userRole, userID,
|
||||
)}
|
||||
|
||||
<div className="flex justify-end space-x-3 pt-6 mt-6 border-t border-gray-200">
|
||||
<Button
|
||||
<Button2
|
||||
onClick={() => {
|
||||
setShowAddCallbacksModal(false);
|
||||
setSelectedCallback(null);
|
||||
@ -691,7 +691,7 @@ const Settings: React.FC<SettingsPageProps> = ({ accessToken, userRole, userID,
|
||||
}}
|
||||
>
|
||||
Cancel
|
||||
</Button>
|
||||
</Button2>
|
||||
<Button2 htmlType="submit">Add Callback</Button2>
|
||||
</div>
|
||||
</Form>
|
||||
|
||||
Loading…
Reference in New Issue
Block a user