mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
fix: remove dead user-cache lookup with None key in spend-update path (#33555)
With litellm_settings.enable_redis_auth_cache enabled, user_api_key_cache is Redis-backed. _update_user_db performed a cache lookup with key=user_id where user_id can be None; the in-memory cache tolerates a None key but Redis raises redis.exceptions.DataError (Invalid input of type: NoneType) on every spend update for requests without a user_id. The looked-up value was never used by any subsequent code, so the lookup is removed along with the user_api_key_cache parameter it existed for. Spend updates for users, end users, and the global proxy budget are unchanged
This commit is contained in:
parent
260d1eae8e
commit
df51cebcd3
2 changed files with 86 additions and 12 deletions
|
|
@ -27,7 +27,7 @@ from typing import (
|
|||
|
||||
import litellm
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
from litellm.caching import DualCache, RedisCache
|
||||
from litellm.caching import RedisCache
|
||||
from litellm.constants import (
|
||||
DB_SPEND_UPDATE_JOB_NAME,
|
||||
DB_DAILY_TAG_SPEND_UPDATE_JOB_NAME,
|
||||
|
|
@ -44,7 +44,6 @@ from litellm.proxy._types import (
|
|||
DailyUserSpendTransaction,
|
||||
DBSpendUpdateTransactions,
|
||||
Litellm_EntityType,
|
||||
LiteLLM_UserTable,
|
||||
SpendLogsMetadata,
|
||||
SpendLogsPayload,
|
||||
SpendUpdateQueueItem,
|
||||
|
|
@ -137,7 +136,6 @@ class DBSpendUpdateWriter:
|
|||
disable_spend_logs,
|
||||
litellm_proxy_budget_name,
|
||||
prisma_client,
|
||||
user_api_key_cache,
|
||||
)
|
||||
from litellm.proxy.utils import ProxyUpdateSpend, hash_token
|
||||
|
||||
|
|
@ -195,7 +193,6 @@ class DBSpendUpdateWriter:
|
|||
org_id=org_id,
|
||||
end_user_id=end_user_id,
|
||||
prisma_client=prisma_client,
|
||||
user_api_key_cache=user_api_key_cache,
|
||||
litellm_proxy_budget_name=litellm_proxy_budget_name,
|
||||
payload=payload,
|
||||
)
|
||||
|
|
@ -326,7 +323,6 @@ class DBSpendUpdateWriter:
|
|||
org_id: Optional[str],
|
||||
end_user_id: Optional[str],
|
||||
prisma_client: Optional[PrismaClient],
|
||||
user_api_key_cache: DualCache,
|
||||
litellm_proxy_budget_name: Optional[str],
|
||||
payload: SpendLogsPayload,
|
||||
):
|
||||
|
|
@ -345,7 +341,6 @@ class DBSpendUpdateWriter:
|
|||
response_cost=response_cost,
|
||||
user_id=user_id,
|
||||
prisma_client=prisma_client,
|
||||
user_api_key_cache=user_api_key_cache,
|
||||
litellm_proxy_budget_name=litellm_proxy_budget_name,
|
||||
end_user_id=end_user_id,
|
||||
)
|
||||
|
|
@ -510,7 +505,6 @@ class DBSpendUpdateWriter:
|
|||
response_cost: Optional[float],
|
||||
user_id: Optional[str],
|
||||
prisma_client: Optional[PrismaClient],
|
||||
user_api_key_cache: DualCache,
|
||||
litellm_proxy_budget_name: Optional[str],
|
||||
end_user_id: Optional[str] = None,
|
||||
):
|
||||
|
|
@ -518,10 +512,6 @@ class DBSpendUpdateWriter:
|
|||
- Update that user's row
|
||||
- Update litellm-proxy-budget row (global proxy spend)
|
||||
"""
|
||||
## if an end-user is passed in, do an upsert - we can't guarantee they already exist in db
|
||||
existing_user_obj = await user_api_key_cache.async_get_cache(key=user_id)
|
||||
if existing_user_obj is not None and isinstance(existing_user_obj, dict):
|
||||
existing_user_obj = LiteLLM_UserTable(**existing_user_obj)
|
||||
try:
|
||||
if prisma_client is not None: # update
|
||||
user_ids = [user_id]
|
||||
|
|
|
|||
|
|
@ -13,7 +13,10 @@ from datetime import datetime, timezone
|
|||
from unittest.mock import AsyncMock, MagicMock, call, patch
|
||||
|
||||
import pytest
|
||||
from redis.exceptions import DataError
|
||||
|
||||
import litellm
|
||||
from litellm.proxy._types import Litellm_EntityType
|
||||
from litellm.proxy.db.db_spend_update_writer import DBSpendUpdateWriter
|
||||
|
||||
|
||||
|
|
@ -1418,7 +1421,6 @@ async def test_batch_database_updates_isolation_on_failure():
|
|||
org_id="org1",
|
||||
end_user_id="eu1",
|
||||
prisma_client=MagicMock(),
|
||||
user_api_key_cache=MagicMock(),
|
||||
litellm_proxy_budget_name="budget",
|
||||
payload={"key": "value"},
|
||||
)
|
||||
|
|
@ -1818,3 +1820,85 @@ async def test_update_database_does_not_deepcopy_on_request_path():
|
|||
fake_payload["nested"]["a"] = 999
|
||||
assert batch_payload["model"] == "gpt-4"
|
||||
assert batch_payload["nested"]["a"] == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_spend_update_path_never_queries_user_cache_with_none_user_id():
|
||||
"""
|
||||
When user_id is None, the spend-update path must not perform a user-cache
|
||||
lookup at all. With a Redis-backed auth cache (enable_redis_auth_cache),
|
||||
a lookup with key=None raises redis.exceptions.DataError, which aborted
|
||||
_update_user_db before any spend updates were enqueued.
|
||||
|
||||
This test fails on the old code twice over: the cache mock records the
|
||||
forbidden lookup, and the DataError it raises kills the end-user spend
|
||||
update that must survive.
|
||||
"""
|
||||
db_writer = DBSpendUpdateWriter()
|
||||
|
||||
strict_redis_backed_cache = MagicMock()
|
||||
strict_redis_backed_cache.async_get_cache = AsyncMock(
|
||||
side_effect=DataError("Invalid input of type: 'NoneType'")
|
||||
)
|
||||
|
||||
with (
|
||||
patch.object(litellm, "max_budget", 0),
|
||||
patch("litellm.proxy.proxy_server.disable_spend_logs", True),
|
||||
patch("litellm.proxy.proxy_server.prisma_client", MagicMock()),
|
||||
patch("litellm.proxy.proxy_server.user_api_key_cache", strict_redis_backed_cache),
|
||||
patch("litellm.proxy.proxy_server.litellm_proxy_budget_name", "litellm-proxy-budget"),
|
||||
patch(
|
||||
"litellm.proxy.spend_tracking.spend_tracking_utils.get_logging_payload",
|
||||
return_value={
|
||||
"startTime": "2024-01-01T00:00:00",
|
||||
"endTime": "2024-01-01T00:01:00",
|
||||
"model": "gpt-4",
|
||||
"custom_llm_provider": "openai",
|
||||
"spend": 0.0,
|
||||
},
|
||||
),
|
||||
):
|
||||
await db_writer.update_database(
|
||||
token=None,
|
||||
user_id=None,
|
||||
end_user_id="end-user-1",
|
||||
team_id=None,
|
||||
org_id=None,
|
||||
kwargs={"model": "gpt-4", "custom_llm_provider": "openai"},
|
||||
completion_response=MagicMock(),
|
||||
start_time=datetime.now(),
|
||||
end_time=datetime.now(),
|
||||
response_cost=0.1,
|
||||
)
|
||||
await asyncio.sleep(0)
|
||||
|
||||
strict_redis_backed_cache.async_get_cache.assert_not_called()
|
||||
|
||||
queued = await db_writer.spend_update_queue.flush_all_updates_from_in_memory_queue()
|
||||
end_user_updates = [u for u in queued if u["entity_type"] == Litellm_EntityType.END_USER]
|
||||
assert len(end_user_updates) == 1
|
||||
assert end_user_updates[0]["entity_id"] == "end-user-1"
|
||||
assert all(u["entity_type"] != Litellm_EntityType.USER for u in queued)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_user_db_enqueues_user_spend_without_cache_dependency():
|
||||
"""
|
||||
_update_user_db needs no cache handle: it enqueues the user spend update
|
||||
(and the end-user one) purely from the ids it is given.
|
||||
"""
|
||||
db_writer = DBSpendUpdateWriter()
|
||||
|
||||
with patch.object(litellm, "max_budget", 0):
|
||||
await db_writer._update_user_db(
|
||||
response_cost=0.25,
|
||||
user_id="user-123",
|
||||
prisma_client=MagicMock(),
|
||||
litellm_proxy_budget_name="litellm-proxy-budget",
|
||||
end_user_id="end-user-9",
|
||||
)
|
||||
|
||||
queued = await db_writer.spend_update_queue.flush_all_updates_from_in_memory_queue()
|
||||
by_type = {u["entity_type"]: u["entity_id"] for u in queued}
|
||||
assert by_type[Litellm_EntityType.USER] == "user-123"
|
||||
assert by_type[Litellm_EntityType.END_USER] == "end-user-9"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue