fix for wrong OpenAIClientOptions used with keyed OpenAIClient(s) - #11003
Merged
Conversation
with keyed OpenAIClients
```
builder.AddKeyedOpenAIClient("openai");
builder.AddKeyedOpenAIClient("ionos");
```
assuming that at least one keyed OpenAIClient has a differnt endpoint,
```
{
"ConnectionStrings": {
"openai": "Key={account_key};"
"ionos": "Endpoint=https://{openai_rest_api_url};Key={account_key};"
}
}
```
getting the client with
```
var client = serviceProvider.GetRequiredKeyedService<OpenAIClient>("ionos")
```
the client uses the key from the connectionstring but it still has the default enpoint https://api.openai.com/v1
The issue is that ConfigureOpenAI ignores the serviceKey and always get's the default OpenAIClientOptions instead of the one created with optionsName = servicekey.
This makes using keyed OpenAIClients virtually impossible because the main reason having multiple OpenAIClients is having different endpoints.
Jürgen Steinblock (Steinblock)
requested review from
Eric Erhardt (eerhardt),
Ankit Jain (radical) and
Sébastien Ros (sebastienros)
as code owners
August 18, 2025 21:39
Contributor
Author
|
@dotnet-policy-service agree |
Member
|
Can you add tests for this? |
Contributor
|
Fixing #9543 options = serviceProvider.GetRequiredService<IOptionsMonitor<OpenAIClientOptions>>()
.Get(serviceKey ?? Options.DefaultName);According to: openai/openai-dotnet#215 |
Contributor
Author
|
Eric Erhardt (@eerhardt) I added a test case that is failing without my changes and passing afterwards. As Kumima (@Kumima) mentioned, testing this is tricky because OpenAIClient doesn't expose the endpoint. I don't really like the test because I usually only test the public interface of a class but I could not figure out how to workaround this so I opted for using reflection to get the endpoint. This would work as well. |
Eric Erhardt (eerhardt)
approved these changes
Aug 19, 2025
Eric Erhardt (eerhardt)
left a comment
Member
There was a problem hiding this comment.
LGTM. Thanks for the fix!
Eric Erhardt (eerhardt)
enabled auto-merge (squash)
August 19, 2025 22:32
Eric Erhardt (eerhardt)
disabled auto-merge
August 19, 2025 22:33
Eric Erhardt (eerhardt)
enabled auto-merge (squash)
August 19, 2025 22:33
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
with keyed OpenAIClients
assuming that at least one keyed OpenAIClient has a differnt endpoint,
getting the client with
the client uses the key from the connectionstring but it still has the default enpoint https://api.openai.com/v1
The issue is that ConfigureOpenAI ignores the serviceKey and always get's the default OpenAIClientOptions instead of the one created with optionsName = servicekey.
This makes using keyed OpenAIClients virtually impossible because the main reason having multiple OpenAIClients is having different endpoints.
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?doc-ideatemplatebreaking-changetemplatediagnostictemplate