[Infra] Optimize CI Pipeline - #23721
Conversation
21 jobs were using xlarge (8 vCPU, 16GB) despite running trivial or low-parallelism workloads. Downgrades 6 trivial container-check/UI jobs to medium (2 vCPU) and 15 lightweight test jobs to large (4 vCPU). Only the 5 high-parallelism mapped test jobs (-n 8/16) remain on xlarge. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Change no_output_timeout from 120m/60m to 15m across all test jobs - Fix multi-line YAML formatting for no_output_timeout entries - Reduce Playwright per-test timeout from 4min to 3min - Add 15s actionTimeout and 30s navigationTimeout to Playwright config Hanging tests now fail in 15min instead of 2hrs, cutting wasted CI time. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Switch from machine VM to docker executor (cimg/node:20.19) - Add npm cache with restore_cache/save_cache - Use npm ci instead of npm install (stop deleting node_modules/package-lock.json) - Enable parallel test execution with --pool forks --maxForks=3 - Bump resource_class to medium+ for 3 vCPUs - Remove unnecessary requires: ui_build dependency Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ui_unit_tests was gated behind ui_build but not required for publishing. This ensures UI unit tests must pass before PyPI publish. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Switch ui_build from machine VM to docker executor (cimg/node:20.19) - Add npm cache (restore_cache/save_cache) keyed on package-lock.json - Use npm ci instead of rm -rf node_modules && npm install - Gate ui_unit_tests behind ui_build (don't run tests if build fails) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…atabase test_bad_database_url only validates DB error handling — it doesn't need the UI. Dockerfile.non_root does a full npm install + next build which is unnecessary overhead for this test. Dockerfile.database skips the UI build (build_admin_ui.sh is a no-op for OSS) making the Docker build much faster. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add restore_cache/save_cache for .next/cache to eliminate the "No build cache found" warning and speed up subsequent Next.js builds. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
build_and_test, helm_chart_testing, and test_bad_database_url each rebuilt Dockerfile.database from scratch (~5-10 min each) despite build_docker_database_image already building and persisting it to workspace. Now all three load the pre-built image via workspace, eliminating 3 redundant Docker builds. Also removes orphaned test_nonroot_image job definition (not referenced in workflow). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…olation - Router testing: add CircleCI parallelism=4 with timing-based test splitting - Guardrails testing: add pytest-xdist -n 4, suppress DEBUG logs with LITELLM_LOG=WARNING - Rewrite conftest.py in both test dirs for xdist compatibility (save/restore pattern) - Fix module-level Router instances in test_router_fallback_handlers, test_router_custom_routing, test_acooldowns_router Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…is state - test_async_fallbacks_streaming: replace deprecated gpt-3.5-turbo fallback with gpt-4o-mini, fix use of module-level kwargs variable - test_ausage_based_routing_fallbacks: remove Redis dependency to prevent shared state across parallel CI containers (test already uses mock_response) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…al API The test was failing because it depended on real API calls to deprecated models. Now uses mock_response to validate streaming through the router without external dependencies. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…allback leak - test_hanging_request_azure: mock httpx.AsyncClient.send to simulate slow response instead of racing real network latency against a 10ms timeout. The old non-existent deployment (gpt-4o-new-test) returned 404 faster than the timeout, causing NotFoundError instead of APITimeoutError. - test_completion_together_ai_llama: update model from deprecated Meta-Llama-3.1-8B-Instruct-Turbo to Llama-3.2-3B-Instruct-Turbo (Together AI removed the old model from serverless). - conftest.py: clear litellm.callbacks list before each test to prevent proxy hooks (SkillsInjectionHook, VirtualKeyModelMaxBudgetLimiter) from leaking across tests via Router initialization. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test relied on a global side effect (customLogger initialization in litellm_logging.py) from prior tests' success callbacks to dispatch failure callbacks. When tests run in parallel by file, no prior test initializes customLogger, so the Router's deployment_callback_on_failure was never invoked and cooldowns were never set. Rewrite to directly call deployment_callback_on_failure with a proper RateLimitError containing retry-after headers, testing the cooldown logic without depending on the logging callback chain. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
test_post_call_rule_streaming in test_rules.py sets litellm.post_call_rules but never cleans up. Since pytest_collection_modifyitems sorts tests by name across modules, the leaked rule causes failures in test_streaming.py, test_register_model.py, and test_sagemaker.py. Add pre_call_rules and post_call_rules to the isolate_litellm_state fixture's save/restore and clear lists. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- test_router_context_window_check_pre_call_check_out_group: replace deprecated gpt-3.5-turbo-1106 (removed from model_cost, returns max_input_tokens=0) with gpt-4.1-mini + mock_response - test_async_fallbacks: filter "Task was destroyed but it is pending" messages that leak from parallel test execution in CI Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add num_retries=0 to the async acompletion call to prevent retries when the mock returns invalid response data. The test only validates request payload format, not retry behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Llama-3.2-3B-Instruct-Turbo is no longer available as a serverless model on Together AI. Switch to Llama-3.3-70B-Instruct-Turbo which is still available and has cost data in the model prices map. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
4 mapped test jobs (llms, core, core_utils, integrations) were running on xlarge with -n 16 workers on 8 vCPUs, causing unnecessary context switching. Downsized to large with worker counts matched to available cores. MCPs job (2 test files) downsized from large to medium. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…DB-cached batch retrieval When a completed batch is served from the DB cache, _hidden_params was empty, causing the managed files hook to skip output_file_id translation from raw provider IDs to unified IDs. This fix populates unified_batch_id and model_id on the early-return path, with a guard against double-encoding when the DB already stores unified IDs. Also reduces file deletion retry delay (20s→5s), reruns (5→2), and CI timeout (30m→15m) to cut worst-case runtime from ~16min to ~4min. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…disable retries Tests failed intermittently in CI (-n 8 workers) because cached AsyncHTTPHandler instances from other tests bypassed the class-level mock on AsyncHTTPHandler.post, causing real requests to OpenAI with mock API keys. Router retries (default 2) masked the root cause. - Add autouse fixture to flush litellm.in_memory_llm_clients_cache before/after each test so mocks always apply to fresh clients - Set num_retries=0 on all Router instances to surface mock failures immediately instead of silently retrying Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ail-fast - Replace real OpenAI/Anthropic/Bedrock API calls with mock_response in ~20 cache tests to eliminate network-dependent flakiness - Remove -x (fail-fast) from caching_unit_tests so all failures are reported - Add parallelism: 2 with circleci tests run --split-by=timings - Improve pip dependency cache key (v2-caching-deps) with fallback key Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…kup instead of hooks The previous approach populated _hidden_params to trigger the managed files hook, but the hook also re-encodes response.id (batch ID), causing double- encoding when the DB already stores unified IDs. Instead, resolve raw output_file_id/error_file_id to unified IDs via a direct DB lookup (same pattern as resolve_input_file_id_to_unified), which avoids the hook entirely. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ui_build (medium → medium+) is on the critical path blocking 3 downstream jobs. ui_unit_tests (medium+ → large, maxForks 3 → 5) targets ~7 min from ~11. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The 4 integration tests were flaky in CI because the AsyncHTTPHandler.post mock was bypassed when aiohttp transport is used. Mock at the higher BaseLLMHTTPHandler.async_response_api_handler level instead, which bypasses the HTTP layer entirely while still exercising router deployment selection, pre-call checks, and response post-processing (item ID rewriting). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The check_batch_cost_job runs on a 10-40s interval and sets batch_processed=True after sending the S3 callback. 30s (6×5s) wasn't enough margin; 50s (10×5s) covers the worst-case poll interval plus processing time, while still being 3.6x faster than the original 180s. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
8 jobs had -n workers set higher than available vCPUs, causing context switch overhead and degraded performance. Aligned -n to match resource_class: - medium (2 CPU): enterprise -n 8→2, image_gen/logging/guardrails -n 4→2 - large (4 CPU): proxy_part1/llms/core/integrations -n 8→4 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- proxy_unit_testing_key_generation: large→medium (serial, 1 test file) - proxy_unit_testing_part1: large→medium, -n 4→2 (only 2 test files) - mapped_tests_proxy_part1: xlarge→large, -n 8→4 (~2000 tests, 4 CPUs sufficient) Saves ~40 credits/min across these 3 jobs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…en tests The isolate_litellm_state conftest fixture saved/restored litellm.callbacks but never cleared it before each test, unlike the other callback lists. It also didn't handle litellm.model_fallbacks. Leaked callbacks and fallback config caused mocked tests to route through Router/fallback paths, hitting real APIs with mock keys. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use ubuntu-2204:2024.04.1 which ships with a recent Docker, eliminating the 1-minute `curl get.docker.com | sh` upgrade step - Switch image save/load from gzip to zstd -1 -T0 for ~3-5x faster compression/decompression, saving ~30s on save and on each downstream load Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test intermittently fails in CI due to Redis cache write propagation delays, causing the second call to miss the cache and hit OpenAI directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Several test jobs were underutilizing their CPU allocation (~25%) because they were either missing pytest-xdist -n or using -n 2 on 4-vCPU machines. Batching individual pip install calls into single commands reduces resolver overhead and saves ~30-60s per job. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Batching pip installs exposed a dependency conflict: langfuse==2.59.7 requires anyio>=4.4.0, which conflicts with the anyio==4.2.0 pin. Dropping the pin lets pip resolve a compatible version. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…stalls Batching pip installs surfaced more hidden conflicts: - respx==0.22.0 requires httpx>=0.25.0, conflicting with httpx==0.24.1 - traceloop-sdk==0.21.1 requires otel-semantic-conventions<0.46, conflicting with opentelemetry-sdk==1.25.0 (needs ==0.46b0) These were masked before because separate pip install calls let later installs silently override earlier pins. Dropping the pins lets pip resolve compatible versions. Verified with pip --dry-run locally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…hing Increase pytest-xdist parallelism to match available CPU on I/O-bound and CPU-bound test jobs. Drop coverage collection from 8 jobs (still collected by ~15 other jobs). Add dependency caching to 4 uncached jobs. Reduce verbose output (-vv to -v) and remove -s/--log-cli-level overhead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Router tests: expand conftest save/restore to cover all globals mutated by router tests (default_fallbacks, tag_budget_config, request_timeout, enable_azure_ad_token_refresh, num_retries_per_request, model_cost, token_counter). These were leaking across xdist workers. Proxy tests: move test_proxy_utils.py (169 parametrized) and test_proxy_server.py (72 parametrized) from part2 to part1, balancing ~370 vs ~360 tests (was ~129 vs ~600). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…, drop unused coverage - Switch setup_litellm_test_deps from pip to uv with batched installs - Cache installed site-packages (~/.local/lib, ~/.local/bin) instead of pip download cache for near-instant installs on cache hit - Remove unused coverage collection from 6 mapped test jobs (only mcps coverage is consumed by the coverage combine step) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…cting pins uv's strict resolver rejects transitive dep conflicts (semantic-router wants openai<2, llm-sandbox wants pydantic>=2.11.5). Use uv for the heavy requirements.txt install and pip for the small test dep batch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Switch pip install -r requirements.txt to uv pip install --system -r requirements.txt for all docker-based jobs that use the main requirements.txt. This applies the same optimization already proven in the mapped test jobs to the rest of the CI pipeline. Also adds --no-deps to semantic_router installs in guardrails_testing and litellm_mapped_enterprise_tests to avoid uv's strict resolution conflict with openai>=2. Skipped: machine executor + conda jobs (security, proxy_spend_accuracy, proxy_multi_instance, proxy_store_model_in_db) and Group B jobs using .circleci/requirements.txt. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…leep The conftest fixtures were saving/restoring the current (potentially contaminated) values of litellm globals like num_retries instead of resetting to true defaults. Under xdist, module-level assignments (e.g. `litellm.num_retries = 3` in 12+ test files) pollute the shared module state and leak across tests in the same worker. - Capture true litellm defaults at conftest import time and reset before each test (local_testing + llm_translation) - Make llm_translation/conftest.py xdist-safe (skip reload, add state isolation) - Replace asyncio.sleep(2) with polling in cooldown handler tests - Add @pytest.mark.flaky to tests making real API calls under xdist Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…e, increase xdist workers Downgrade langfuse, assistants, and python 3.13 install jobs to medium (were defaulting to large at ~25% CPU). Bump enterprise and image_gen xdist workers to -n 4 on explicit large instances. Drop coverage collection and persist_to_workspace for 4 jobs that no longer need it. Downgrade verbosity from -vv to -v across all 5 jobs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…alls, reduce verbosity - caching_unit_tests: add resource_class large, enable xdist -n 4, drop unused coverage collection - build_and_test & proxy_pass_through_endpoint_tests: remove redundant Docker CLI install (machine executor has it) - Downgrade -vv to -v across 4 jobs to reduce log noise Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ead of sleep" This reverts commit 9711e3a.
…ls and enterprise tests semantic_router imports aurelio_sdk at module load time, so it must be installed even when using --no-deps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ocker xlarge Switch from expensive Linux machine (medium) to docker xlarge executor. Drop miniconda, manual Docker CLI install, and manual PostgreSQL container in favor of cimg/python:3.13, setup_remote_docker, and service container. Use uv + cache for dependency installation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
aurelio_sdk imports requests_toolbelt at load time, so it needs its deps. Unlike semantic_router, aurelio_sdk has no conflict with openai>=2, so --no-deps is unnecessary. Verified via uv dry-run locally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…oldowns The revert of 9711e3a left xdist tests without proper state isolation. Module-level assignments like `litellm.num_retries = 3` in 12+ test files pollute shared globals, and the fixture was saving/restoring contaminated values instead of resetting to true defaults. - Capture true litellm defaults at conftest import time and reset before each test (local_testing + llm_translation) - Make llm_translation/conftest.py xdist-safe (skip reload under xdist, add state isolation) - Replace asyncio.sleep(2) with polling in cooldown handler tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…leakage test_rerank.py sets litellm.api_base = "http://localhost:4000" which leaked to all subsequent tests on the same xdist worker, causing connection failures across every provider (Cohere, Azure, OpenAI, etc.). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nection churn The old conftest never flushed HTTP client cache. Adding flush_cache() before every test forces new TCP connections to external APIs, causing transient connection failures under xdist parallelism. Global state isolation is already handled by _SCALAR_DEFAULTS reset. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ures GCS cache tests (test_gcs_cache_unit_tests.py) rely on module-level state (vertex_chat_completion singleton, credential caches) that importlib.reload resets but the xdist-safe function-scoped fixture does not. Removing -n 4 from this job restores single-process execution where module reload properly resets all state before each test, while CI-level parallelism (parallelism: 2) still splits test files across nodes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…test The xdist-conditional reload (manual reset in xdist mode) was missing attributes that importlib.reload resets, causing Azure connection errors. The original conftest used importlib.reload unconditionally (even under xdist) and that worked on main. Restore that behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Part1 had 4 test files combined (was originally 2), causing cross-file state pollution under xdist. Reverted to original grouping. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Resolved conflict in test_claude_agent_sdk.py by keeping main's additions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR tackles CI pipeline speed (from ~60+ mins → ~11 mins) through three orthogonal efforts: (1) a CircleCI infra overhaul using Key changes:
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| .circleci/config.yml | Major CI pipeline overhaul: uses uv for faster installs, adds parallelism/resource_class tweaks, reduces no_output_timeout from 120m→15m, drops several explicit version pins (httpx, anyio, opentelemetry-*) from proxy jobs relying on requirements.txt coverage instead |
| litellm/proxy/openai_files_endpoints/common_utils.py | Adds resolve_output_file_ids_to_unified helper and eagerly sets batch_processed=True in update_batch_in_database on completion, with a fallback retry path for missing schema columns |
| litellm/proxy/batches_endpoints/endpoints.py | Adds "complete" to the terminal batch status check and calls the new resolve_output_file_ids_to_unified to resolve raw provider output/error file IDs on retrieve_batch |
| enterprise/litellm_enterprise/proxy/common_utils/check_batch_cost.py | Removes "complete"/"completed" from status exclusion list, relying solely on batch_processed=False filter to prevent reprocessing; allows CheckBatchCost to catch batches that transitioned before it ran |
| tests/proxy_e2e_anthropic_messages_tests/test_claude_agent_sdk.py | Extracts _run_streaming_test helper and adds a 3-retry loop, but the original test body (test_query, received_chunks, full_response, try/except) was left as dead code after pytest.fail() |
| tests/test_litellm/test_eager_tiktoken_load.py | Replaces in-process module reload (which contaminated xdist workers) with subprocess execution via _run_python; removes the now-unnecessary autouse cleanup fixture |
| tests/test_litellm/router_utils/pre_call_checks/test_encrypted_content_affinity_check.py | Replaces low-level AsyncHTTPHandler.post mock with BaseLLMHTTPHandler.async_response_api_handler mock and uses typed ResponsesAPIResponse objects; adds num_retries=0 to routers for deterministic test behaviour |
| tests/local_testing/conftest.py | Adds function-scoped isolate_litellm_state fixture that saves/restores litellm globals before/after each test; demotes litellm reload to module-scope and skips it under xdist |
| tests/logging_callback_tests/conftest.py | Same xdist-compatible isolation pattern as local_testing/conftest.py; captures true defaults at import time and resets before/after each test; clears _in_memory_loggers to prevent logger instance leaks |
| tests/local_testing/test_router.py | Rewrites test_aaarouter_dynamic_cooldown_message_retry_time to use direct deployment_callback_on_failure calls instead of patching the OpenAI client; removes real network dependency |
Sequence Diagram
sequenceDiagram
participant C as Client
participant RB as retrieve_batch endpoint
participant DB as Database (ManagedObjectTable)
participant CB as Async Cost Callback (@client)
participant CBC as CheckBatchCost (background)
C->>RB: GET /batch/{id}
RB->>DB: get_batch_from_database(batch_id)
DB-->>RB: batch (status="complete" or still in-progress)
alt batch already terminal in DB
RB->>RB: resolve_input_file_id_to_unified()
RB->>RB: resolve_output_file_ids_to_unified() [NEW]
RB-->>C: return cached response
else batch not yet terminal
RB->>RB: call provider API (aretrieve_batch)
RB->>DB: update_batch_in_database(status="complete")
Note over DB: Sets batch_processed=True [NEW]<br/>Fallback: retry without flag if column missing
DB-->>RB: updated
RB->>CB: asyncio.create_task(cost_callback) [fire-and-forget]
RB-->>C: return response
end
CB-->>DB: record batch cost (async, best-effort)
Note over CBC: Polls periodically
CBC->>DB: find_many(status NOT IN [failed,expired,cancelled,stale_expired],<br/>batch_processed=False) [CHANGED: complete no longer excluded]
DB-->>CBC: unprocessed batches
CBC->>CBC: calculate & record cost
CBC->>DB: set batch_processed=True
Comments Outside Diff (1)
-
.circleci/config.yml, line 204-205 (link)Aggressive
no_output_timeoutreduction may cause flaky CIThe
no_output_timeoutfor both proxy-test jobs is dropped from 120 minutes to 15 minutes. The PR author reports ~11 minute runs on the linked pipeline, giving a 4-minute buffer. That buffer may be too thin if:- Dependency installation is a cache miss (uv still needs to download packages)
- A slow/flaky test causes a single xdist worker to stall
Consider bumping to
30mas a more comfortable safety margin while still being far below the old 120-minute setting. The same consideration applies to thecaching_unit_testsand other jobs that were also reduced.
Last reviewed commit: 8f56ddb
| @@ -90,31 +171,31 @@ async def test_claude_agent_sdk_streaming(litellm_proxy_config, model_name, mode | |||
| chunk_text = msg.content_block.text | |||
| received_chunks.append(chunk_text) | |||
| full_response += chunk_text | |||
|
|
|||
| # Fallback to content handling | |||
| if hasattr(msg, 'content'): | |||
| for content_block in msg.content: | |||
| if hasattr(content_block, 'text'): | |||
| chunk_text = content_block.text | |||
| received_chunks.append(chunk_text) | |||
| full_response += chunk_text | |||
|
|
|||
| # Assertions | |||
| print(f"\n✅ Received {len(received_chunks)} chunks") | |||
| print(f"📝 Full response: {full_response[:100]}...") | |||
|
|
|||
| # Verify we got a response | |||
| assert len(full_response) > 0, f"No response received from {model_name}" | |||
|
|
|||
| # Verify streaming (should have multiple chunks for most responses) | |||
| # Note: Very short responses might come in 1 chunk, so we just verify we got content | |||
| assert len(received_chunks) > 0, f"No chunks received from {model_name}" | |||
|
|
|||
| # Verify response is non-empty (don't assert on specific LLM content — it's non-deterministic) | |||
| assert len(full_response.strip()) > 0, f"Empty response received from {model_name}" | |||
|
|
|||
| print(f"✅ Test passed for {model_name}") | |||
|
|
|||
| except Exception as e: | |||
| pytest.fail(f"Test failed for {model_name} ({model_description}): {str(e)}") | |||
|
|
|||
There was a problem hiding this comment.
Dead code left after refactor
After pytest.fail(...) raises a Failed exception (it never returns), the entire original test body — test_query, received_chunks, full_response, the try/except block, and the assertions on full_response.strip() — is unreachable dead code. The logic was correctly moved into _run_streaming_test, but the old body was never removed.
Additionally, options (a ClaudeAgentOptions object) is referenced inside the dead block but was only defined inside _run_streaming_test, so if this code were somehow reached it would raise a NameError.
The lines from # Test query onwards should be deleted:
pytest.fail(
f"Test failed for {model_name} ({model_description}) after {MAX_RETRIES} attempts: {last_error}"
)
# ↑ Everything below this line is dead code and should be removed.| 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 |
There was a problem hiding this comment.
CheckBatchCost safety-net comment is misleading
The inline comment says:
CheckBatchCostacts as a safety net for the rare case where the callback fails.
However, batch_processed=True is set here in update_batch_in_database before the async cost callback has a chance to run. Because CheckBatchCost now queries WHERE batch_processed = FALSE, any batch for which update_batch_in_database succeeds (i.e., the column exists and the write works) will have batch_processed=True and will not be picked up by CheckBatchCost, regardless of whether the cost callback later fails.
The true safety net only applies when update_batch_in_database itself fails to persist batch_processed (e.g., the column is absent and the fallback retry also errors). Consider updating the comment to accurately reflect this narrower guarantee, so future maintainers aren't misled into thinking cost-callback failures are automatically retried:
# Setting batch_processed=True here unblocks file deletion (which
# queries batch_processed=False). NOTE: this flag is set eagerly,
# so CheckBatchCost is only a safety net for the case where this
# update itself fails (e.g. old schema without the column). If the
# async cost callback fails after this write succeeds, the cost will
# NOT be re-calculated automatically.
if db_status == "complete":
update_data["batch_processed"] = True[Infra] Optimize CI Pipeline
Relevant issues
CI/CD now runs in ~11 mins
https://app.circleci.com/pipelines/github/BerriAI/litellm?branch=litellm_ci_optimize
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Type
🚄 Infrastructure
Changes