Repository navigation
Conversation
Embedding clients were left out of the deterministic lifecycle that microsoft#9748 gave chat clients: BaseEmbeddingClient had no __aenter__/__aexit__/close(), so 'async with' did not work and internally created SDK clients (OpenAI httpx pools, Ollama httpx transports) were only released by GC. This aligns the embedding family with the chat clients and with the existing GeminiEmbeddingClient.close()/_owns_client pattern. - BaseEmbeddingClient: async context manager and a no-op close() default, mirroring BaseChatClient. - OpenAI embedding client: track _owns_client; close() awaits client.close() only when the SDK client was created internally. - Ollama embedding client: same ownership pattern via AsyncClient.aclose(). Caller-supplied clients always stay open: their lifecycle belongs to the caller. Bedrock's boto3 client has no close() to call and keeps its default no-op. Closes microsoft#9814
Lushiyao5
requested review from
SergeyMenshykh,
Eduard van Valkenburg (eavanvalkenburg),
Roger Barreto (rogerbarreto) and
westey (westey-m)
as code owners
October 30, 2026 17:23
Lushiyao5
temporarily deployed
to
github-app-auth
October 30, 2026 17:23 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Lushiyao5
requested review from
Darren Cohen (dargilco),
Jose Alvarez (jpalvarezl) and
sophia-ramsey
as code owners
October 30, 2026 17:23
agent-framework-automation
Bot
added
the
python
github-actions
Bot
changed the title
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Internally created Ollama clients raise AttributeError during cleanup because AsyncClient has no close() method, leaving the HTTP transport open.
1 open issue
What changed in this PR
Extends the Python chat-client lifecycle pattern to embedding clients, adding optional context management and ownership-aware cleanup to address #9814.
Changes:
- Adds async context management and a default no-op
close()toBaseEmbeddingClient. - Adds ownership tracking and cleanup methods to OpenAI and Ollama embedding clients.
- Adds tests for the base lifecycle and OpenAI client ownership.
| File | Description |
|---|---|
python/packages/openai/tests/openai/test_openai_embedding_client.py |
Tests cleanup of owned clients and preservation of caller-supplied clients. |
python/packages/openai/agent_framework_openai/_embedding_client.py |
Adds ownership-aware OpenAI SDK cleanup. |
python/packages/ollama/agent_framework_ollama/_embedding_client.py |
Adds ownership tracking and a cleanup override. |
python/packages/core/tests/core/test_embedding_client.py |
Tests context entry and default no-op cleanup. |
python/packages/core/agent_framework/_clients.py |
Defines the shared embedding-client lifecycle methods. |
Review effort: Balanced
| A caller-supplied ``client`` stays open: its lifecycle belongs to the caller. | ||
| """ | ||
| if self._owns_client: | ||
| await self.client.close() |
Author
|
@microsoft-github-policy-service agree |
Member
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #9814
Embedding clients were left out of the deterministic lifecycle that #9748 gave chat clients:
BaseEmbeddingClienthad no__aenter__/__aexit__/close(), soasync withdid not work for any embedding client, and internally created SDK clients (OpenAI httpx connection pools, Ollama httpx transports) were only ever released by GC.Change
BaseEmbeddingClient(core): async context manager plus a no-opclose()default, mirroringBaseChatClient— entering the context is never required for ordinary requests, and resource-owning implementations overrideclose().OpenAIEmbeddingClient: tracks_owns_client = async_client is None;close()awaitsclient.close()only when the SDK client was created internally (viaload_openai_service_settings), covering both the OpenAI and Azure paths.OllamaEmbeddingClient: the same ownership pattern;close()awaitsAsyncClient.aclose().Caller-supplied clients always stay open — their lifecycle belongs to the caller.
GeminiEmbeddingClientalready implemented this pattern and is untouched. Bedrock's boto3 runtime client has no close() surface, so it keeps the base no-op.Tests
test_embedding_client.py): the async context manager protocol works on the base and the defaultclose()is a no-op;test_openai_embedding_client.py):close()releases an internally created client and leaves a caller-supplied one unawaited;