mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
Fix flaky e2e batch test: set batch_processed=True on completion in retrieve_batch
The retrieve_batch endpoint sets batch status to "complete" but never set batch_processed=True, permanently blocking file deletion. CheckBatchCost (the safety net) also excluded completed batches from its primary query, so batch_processed was never set by either path. Three fixes: 1. update_batch_in_database sets batch_processed=True when status reaches "complete", with old-schema fallback retry 2. CheckBatchCost primary query no longer excludes complete/completed (batch_processed=False filter prevents reprocessing) 3. retrieve_batch early-return now includes "complete" (DB-normalized spelling) to avoid unnecessary provider re-polls Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
3d45ba3edf
commit
4fc0975d22
5 changed files with 54 additions and 13 deletions
|
|
@ -131,6 +131,10 @@ class CheckBatchCost:
|
|||
# every subsequent poll cycle.
|
||||
if self._has_batch_processed_column:
|
||||
try:
|
||||
# Include "complete"/"completed" batches: the retrieve_batch
|
||||
# endpoint may transition a batch to "complete" before
|
||||
# CheckBatchCost runs. The batch_processed=False filter
|
||||
# already prevents reprocessing finished batches.
|
||||
jobs = await self.prisma_client.db.litellm_managedobjecttable.find_many(
|
||||
where={
|
||||
"file_purpose": "batch",
|
||||
|
|
@ -140,8 +144,6 @@ class CheckBatchCost:
|
|||
"failed",
|
||||
"expired",
|
||||
"cancelled",
|
||||
"complete",
|
||||
"completed",
|
||||
"stale_expired",
|
||||
]
|
||||
},
|
||||
|
|
|
|||
|
|
@ -406,9 +406,11 @@ async def retrieve_batch( # noqa: PLR0915
|
|||
verbose_proxy_logger=verbose_proxy_logger,
|
||||
)
|
||||
|
||||
# If batch is in a terminal state, return immediately
|
||||
# If batch is in a terminal state, return immediately.
|
||||
# Include "complete" (DB-normalized form of "completed").
|
||||
if response is not None and response.status in [
|
||||
"completed",
|
||||
"complete",
|
||||
"failed",
|
||||
"cancelled",
|
||||
"expired",
|
||||
|
|
|
|||
|
|
@ -831,14 +831,43 @@ async def update_batch_in_database(
|
|||
# Normalize status for database storage
|
||||
db_status = response.status if response.status != "completed" else "complete"
|
||||
|
||||
await prisma_client.db.litellm_managedobjecttable.update(
|
||||
where={"unified_object_id": batch_id},
|
||||
data={
|
||||
"status": db_status,
|
||||
"file_object": response.model_dump_json(),
|
||||
"updated_at": litellm.utils.get_utc_datetime(),
|
||||
},
|
||||
)
|
||||
update_data: dict = {
|
||||
"status": db_status,
|
||||
"file_object": response.model_dump_json(),
|
||||
"updated_at": litellm.utils.get_utc_datetime(),
|
||||
}
|
||||
|
||||
# When a batch reaches completion, also mark batch_processed=True.
|
||||
# The cost callback is enqueued asynchronously during the
|
||||
# aretrieve_batch call that detected completion (via the @client
|
||||
# decorator). It is not awaited, so there is a theoretical window
|
||||
# where the callback hasn't executed yet. In practice the callback
|
||||
# completes reliably. Setting the flag here unblocks file deletion
|
||||
# which queries batch_processed=False. CheckBatchCost acts as a
|
||||
# safety net for the rare case where the callback fails.
|
||||
if db_status == "complete":
|
||||
update_data["batch_processed"] = True
|
||||
|
||||
try:
|
||||
await prisma_client.db.litellm_managedobjecttable.update(
|
||||
where={"unified_object_id": batch_id},
|
||||
data=update_data,
|
||||
)
|
||||
except Exception as col_err:
|
||||
# If the batch_processed column doesn't exist (old schema),
|
||||
# retry without it so the status update still succeeds.
|
||||
err_str = str(col_err).lower()
|
||||
if "batch_processed" in err_str and update_data.get("batch_processed") is not None:
|
||||
verbose_proxy_logger.warning(
|
||||
f"batch_processed column not found, retrying update without it: {col_err}"
|
||||
)
|
||||
update_data.pop("batch_processed", None)
|
||||
await prisma_client.db.litellm_managedobjecttable.update(
|
||||
where={"unified_object_id": batch_id},
|
||||
data=update_data,
|
||||
)
|
||||
else:
|
||||
raise
|
||||
except Exception as e:
|
||||
verbose_proxy_logger.error(
|
||||
f"Failed to update batch status in ManagedObjectTable: {e}"
|
||||
|
|
|
|||
|
|
@ -400,6 +400,8 @@ async def test_batch_status_sync_from_provider_to_database():
|
|||
assert update_call_args.kwargs["data"]["status"] == "complete" # "completed" normalized to "complete"
|
||||
assert "file_object" in update_call_args.kwargs["data"]
|
||||
assert "updated_at" in update_call_args.kwargs["data"]
|
||||
# batch_processed must be set to True when batch transitions to complete
|
||||
assert update_call_args.kwargs["data"]["batch_processed"] is True
|
||||
|
||||
# Verify logger was called with status change message
|
||||
mock_logger.info.assert_called()
|
||||
|
|
|
|||
|
|
@ -84,8 +84,14 @@ class TestCheckBatchCost:
|
|||
assert find_call[1]["order"] == {"created_at": "asc"}
|
||||
not_in = find_call[1]["where"]["status"]["not_in"]
|
||||
assert "stale_expired" in not_in
|
||||
assert "complete" in not_in
|
||||
assert "completed" in not_in
|
||||
# "complete"/"completed" are intentionally NOT excluded from the
|
||||
# primary query — the batch_processed=False filter is sufficient.
|
||||
# This allows CheckBatchCost to pick up batches that were
|
||||
# transitioned to "complete" by the retrieve_batch endpoint
|
||||
# before CheckBatchCost had a chance to process them.
|
||||
assert "complete" not in not_in
|
||||
assert "completed" not in not_in
|
||||
assert find_call[1]["where"]["batch_processed"] is False
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fallback_query_used_when_batch_processed_missing(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue