Commit graph

6 commits

Author SHA1 Message Date
yuneng-jiang
6a0d03914c
test: drop the cwd-relative sys.path.insert calls from the test suite (#37802)
* test: drop the cwd-relative sys.path.insert calls from the test suite

TQ003 stands at 1,077 across 1,058 files, and 1,015 of them are the same shape:
sys.path.insert(0, os.path.abspath("../..")) and its deeper siblings. The
argument resolves against the working directory rather than the file, so from
the repo root, where every job runs pytest, it inserts the directory two levels
above the checkout. It has never pointed at litellm. The package is installed
into the environment anyway, which is what actually makes the import work, and
what the rule's message has said all along.

Removing them leaves 1,634 imports of sys and os with no remaining reference,
and those go too, except where another test module imports the name back out of
the file. The rest of TQ003 is 62 call sites that resolve against __file__ or a
variable, which are a different question and are left alone.

Collection is identical either way: 45,871 tests and the same 51 pre-existing
collection errors before and after, and ruff reports no new undefined name.

* test: drop the duplicate imports the sys.path sweep exposed to F811

* test(pre-call-utils): restore the os import the new bedrock tests need
2026-08-22 09:25:58 -07:00
mateo-berri
f95367db5f Revert "revert: "fix(caching): close evicted LLM clients so their connections are reclaimed (#35492)""
This reverts commit adb9a53ba1.
2026-08-04 18:23:06 -07:00
mateo-berri
adb9a53ba1 revert: "fix(caching): close evicted LLM clients so their connections are reclaimed (#35492)"
This reverts commit 66bc70365f and the
follow-up 2-line type fix a6d4654261 (#35706), which only retyped a
signature #35492 introduced.

Closing evicted litellm-owned clients breaks every object that fetches
get_async_httpx_client once in __init__ and holds the handler for the
life of the process: 40 guardrail classes plus the pagerduty and email
callbacks. Once the cache entry is evicted (TTL 3600s or the 200-entry
size cap) and the 900s grace passes, the held client is closed and every
subsequent request through it fails with RuntimeError: Cannot send a
request, as the client has been closed. On a production deployment with
a default-on guardrail this surfaced as every request 500ing roughly 75
minutes after boot.

The connection-reclaim goal of #35492 can re-land once handlers survive
their inner client being closed.
2026-08-04 17:05:06 -07:00
Yassin Kortam
66bc70365f
fix(caching): close evicted LLM clients so their connections are reclaimed (#35492)
An evicted client was left for the garbage collector, but every OpenAI/Azure
SDK client is a reference cycle, so nothing freed the client or its pooled TCP
connections until a generational sweep ran. Driving 2000 azure calls through
the official image with no forced collection, live clients and open sockets
climbed from 202 to 1361 while the cache stayed at its 200-entry bound, and RSS
grew 279 MB to 456 MB against a TLS upstream.

Closing on eviction is what caused the earlier 'Cannot send a request, as the
client has been closed' regression, so an evicted client litellm created is now
closed only once a grace window has passed, by which point any request that was
already holding it has finished. A client the caller supplied is never closed,
since litellm does not own its lifecycle.

Resolves LIT-4883
2026-08-03 13:28:38 -07:00
Ishaan Jaff
503eb2fd4c
fix: don't close HTTP/SDK clients on LLMClientCache eviction (#22925)
* fix: don't close HTTP/SDK clients on LLMClientCache eviction

Removing the _remove_key override that eagerly called aclose()/close()
on evicted clients. Evicted clients may still be held by in-flight
streaming requests; closing them causes:

  RuntimeError: Cannot send a request, as the client has been closed.

This is a regression from commit fb72979432. Clients that are no longer
referenced will be garbage-collected naturally. Explicit shutdown cleanup
happens via close_litellm_async_clients().

Fixes production crashes after the 1-hour cache TTL expires.

* test: update LLMClientCache unit tests for no-close-on-eviction behavior

Flip the assertions: evicted clients must NOT be closed. Replace
test_remove_key_closes_async_client → test_remove_key_does_not_close_async_client
and equivalents for sync/eviction paths.

Add test_remove_key_removes_plain_values for non-client cache entries.
Remove test_background_tasks_cleaned_up_after_completion (no more _background_tasks).
Remove test_remove_key_no_event_loop variant that depended on old behavior.

* test: add e2e tests for OpenAI SDK client surviving cache eviction

Add two new e2e tests using real AsyncOpenAI clients:
- test_evicted_openai_sdk_client_stays_usable: verifies size-based eviction
  doesn't close the client
- test_ttl_expired_openai_sdk_client_stays_usable: verifies TTL expiry
  eviction doesn't close the client

Both tests sleep after eviction so any create_task()-based close would
have time to run, making the regression detectable.

Also expand the module docstring to explain why the sleep is required.

* docs(AGENTS.md): add rule — never close HTTP/SDK clients on cache eviction

* docs(CLAUDE.md): add HTTP client cache safety guideline
2026-03-05 12:00:38 -08:00
Shivaang
fb72979432 fix(caching): store background task references in LLMClientCache._remove_key to prevent unawaited coroutine warnings
Fixes #22128
2026-02-27 21:23:56 -05:00