-
Notifications
You must be signed in to change notification settings - Fork 798
Remove non-AsXx surface area from M.E.AI.OpenAI/AzureAIInference #6138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
src/Libraries/Microsoft.Extensions.AI.AzureAIInference/AzureAIInferenceChatClient.cs
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMapper.ChatMessage.cs
Show resolved
Hide resolved
This is certainly all good stuff in general. Within the |
src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIRealtimeExtensions.cs
Show resolved
Hide resolved
c65858f
to
6f5872d
Compare
This should be ready to go. One important note:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving for the single change in the eval test 👍🏾
This seems right to me, though it will also force a change in the Aspire packages. Right now we have: builder.AddOpenAIClient(serviceName).AddChatClient(); ... in which We'll have to make a call about whether to let Aspire integration continue defaulting to the chat API or whether to redesign it so there isn't a specific default. And if there's no default, whether it needs to be controllable from the hosting layer or not (e.g., by having separate Aspire resource types or some other flag that flows through). |
test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAIResponseClientTests.cs
Show resolved
Hide resolved
We expect the AsChatClient/AsEmbeddingGenerator extension methods from M.E.AI.OpenAI and M.E.AI.AzureAIInference to move into the OpenAI / Azure.AI.Inference libraries, respectively. To prepare for that, and the temporary M.E.AI.OpenAI/AzureAIInference libs then being deprecated, this PR removes the other surface area from these assemblies. For anything folks found useful, we should find another way to ship it, likely just as sample source somewhere.
756e5c2
to
56b1114
Compare
@SteveSandersonMS, @jeffhandley, per my discussion with Steve yesterday, I temporarily added back the AsChatClient(OpenAIClient) and AsEmbeddingGenerator(OpenAIClient) extensions, but as EBNever and Obsolete. |
Only meaningful code changes are deleting the extension methods on OpenAIClient (keeping the ones on ChatClient / EmbeddingClient), and renaming the AsChatClient / AsEmbeddingGenerator extension methods to be AsIChatClient / AsIEmbeddingGenerator. Otherwise, "ChatClient" is confusing with the OpenAI type of the same name.
56b1114
to
41d63f0
Compare
We expect the AsChatClient/AsEmbeddingGenerator extension methods from M.E.AI.OpenAI and M.E.AI.AzureAIInference to move into the OpenAI / Azure.AI.Inference libraries, respectively. To prepare for that, and the temporary M.E.AI.OpenAI/AzureAIInference libs then being deprecated, this PR removes the other surface area from these assemblies. For anything folks found useful, we should find another way to ship it, likely just as sample source somewhere.
Closes #6094
Closes #5965
Closes #5964
Microsoft Reviewers: Open in CodeFlow