Conversation
Proposal to fix #3913 — cached_async_http_client uses @functools.cache with no event-loop awareness, causing RuntimeError in multi-worker envs. Three-layer approach: 1. Replace cache with factory (create_async_http_client) 2. Add __aenter__/__aexit__ to Provider base class 3. Thread lifecycle through Agent → Model → Provider Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Docs Preview
|
|
|
||
| def _with_http_client(provider: Provider[Any]) -> Provider[Any]: | ||
| if own_http_client: | ||
| provider._own_http_client = http_client # pyright: ignore[reportPrivateUsage] |
There was a problem hiding this comment.
This helper reaches into the provider's private _own_http_client from outside the class. This happens because the gateway creates the httpx client and passes it to provider constructors in a way that hits the "user-provided client" branch (e.g. AnthropicProvider(anthropic_client=AsyncAnthropic(..., http_client=http_client))), so the provider constructor doesn't know it should claim ownership.
Consider whether providers should accept a parameter to indicate ownership (e.g. an internal _http_client_owned: bool flag, or a dedicated constructor path), so external callers don't need to mutate private state. @DouweM
|
A few high-level notes: Issue reference: The PR body doesn't reference the issue this implements (#3913). Please add Documentation: This PR adds new public behavior — providers and models are now async context managers, and Missing Resource leak in Good progress on a tricky refactor — the core idea of replacing the process-global cache with per-provider ownership is sound and well-aligned with DouweM's guidance on #3913. |
|
|
||
|
|
||
| def cached_async_http_client( | ||
| def create_async_http_client( |
There was a problem hiding this comment.
The provider parameter in this new public function is dead code — it's not passed to httpx.AsyncClient() and there's no caching to use it as a discriminator anymore. Since this is a brand new function (not a deprecated alias), now is the cleanest time to remove it rather than shipping an unused parameter in the public API that users will wonder about.
If there's value in keeping it for debugging/logging (e.g. custom User-Agent per provider, or as a label in error messages), it should actually do something. Otherwise, remove it and let the test fixture use its own caching key that isn't coupled to the public API signature.
cached_async_http_clientwithcreate_async_http_client