dotnet / aspire

Tools, templates, and packages to accelerate building observable, production-ready apps
https://learn.microsoft.com/dotnet/aspire
MIT License
3.81k stars 453 forks source link

Add OpenAI component #4567

Closed eerhardt closed 1 month ago

eerhardt commented 4 months ago

Today we have the Azure.AI.OpenAI component when using Azure OpenAI.

Last week an official OpenAI library was introduced with https://devblogs.microsoft.com/dotnet/openai-dotnet-library/. We should consider having a plain Aspire.OpenAI component that wraps this library for the cases where an Aspire app wants to use OpenAI, but doesn't want to bring in Azure dependencies.

cc @sebastienros @tg-msft @davidfowl @mitchdenny

mitchdenny commented 4 months ago

Are you proposing that we introduce another hosting package as well Aspire.Hosting.OpenAI? It probably doesn't give us much over AddConnectionString other than discoverability.

mitchdenny commented 4 months ago

Adding to the backlog because we'll definitely be doing this.

eerhardt commented 4 months ago

Are you proposing that we introduce another hosting package as well Aspire.Hosting.OpenAI?

Not necessarily "OpenAI" exactly. But I could imagine we could have Aspire.Hosting.Ollama (and others) that ran in a container and exposed an OpenAI endpoint, which was connected to by the Aspire.OpenAI component.

Since OpenAI itself is an existing service that you don't run locally, or provision, I'm not sure what an Aspire.Hosting.OpenAI hosting package would do. But I'm interested in hearing other's thoughts.

eerhardt commented 4 months ago

cc @timheuer @luisquintanilla in case they have any thoughts around here.

luisquintanilla commented 4 months ago

Not sure how it maps out but there's a few things to consider:

Hosting

Client

The configuration depends on which service you're using. However, you might be able to simplify things by installing Azure.OpenAI package since that takes a dependency on OpenAI. Therefore, you get access to all underlying types:

OpenAI

var client= OpenAIClient(...);

Azure OpenAI

var client = AzureOpenAIClient(...);

Consumption

Given one of these clients for the respective service, users can now instantiate a client based on the task (chat / embedding / audio / images/ assistants)

For example, chat:

var chatClient = client.GetChatClient(...);

This works with either OpenAIClient or AzureOpenAIClient.

When using the SDKs directly, users would have to create the respective clients based on what they're looking to do.

Semantic Kernel is currently working on updating their abstractions to the most recent versions of the OpenAI libraries, but I suspect the way it'll work is, given an OpenAI / Azure OpenAI client (this is probably what the Aspire component would register for DI), the respective IChatCompletionService, IEmbeddingGenerationService, and other abstractions would handle the creation / management of task-specific clients.

https://github.com/microsoft/semantic-kernel/issues/6738

cc: @stephentoub @RogerBarreto

stephentoub commented 4 months ago

but I suspect the way it'll work is, given an OpenAI / Azure OpenAI client (this is probably what the Aspire component would register for DI), the respective IChatCompletionService, IEmbeddingGenerationService, and other abstractions would handle the creation / management of task-specific clients.

Yes.

Right now for example the AddOpenAIChatCompletion extension method supports querying DI for an OpenAIClient. The intent with the revised extensions based on the new OpenAI / Azure.AI.OpenAI libraries is to have AddOpenAIChatCompletion continue to look for an OpenAIClient and AddAzureOpenAIChatCompletion look for an AzureOpenAIClient. But obviously this can adapt based on what we collectively decide to do here with Aspire; whatever we do, we want to make sure it's a seamless experience up and down the stack.

eerhardt commented 4 months ago

The intent with the revised extensions based on the new OpenAI / Azure.AI.OpenAI libraries is to have AddOpenAIChatCompletion continue to look for an OpenAIClient and AddAzureOpenAIChatCompletion look for an AzureOpenAIClient.

One approach Aspire could take w.r.t. to DI is:

That way someone just looking for an OpenAIClient can bind to either, and people can switch between Azure and non-Azure, if they want. And if someone is specifically looking for an AzureOpenAIClient, it will only work when using Aspire.Azure.AI.OpenAI.

stephentoub commented 4 months ago

an AzureOpenAIClient service to DI and the same object as an OpenAIClient service

I was hypothesizing in a discussion earlier today that this is what we'd end up doing, assuming the abstraction holds up and everything available via the base OpenAIClient fully works when the concrete type is AzureOpenAIClient. @trrwilson, I assume that's the goal and anything that deviated from that would be a bug to be fixed?

trrwilson commented 4 months ago

an AzureOpenAIClient service to DI and the same object as an OpenAIClient service

I was hypothesizing in a discussion earlier today that this is what we'd end up doing, assuming the abstraction holds up and everything available via the base OpenAIClient fully works when the concrete type is AzureOpenAIClient. @trrwilson, I assume that's the goal and anything that deviated from that would be a bug to be fixed?

Right, with the exception of non-overlapping capabilities; e.g. you won't be able to get/use a ModerationClient from AzureOpenAIClient because Azure OpenAI doesn't have a standalone /moderations API.

stephentoub commented 4 months ago

an AzureOpenAIClient service to DI and the same object as an OpenAIClient service

I was hypothesizing in a discussion earlier today that this is what we'd end up doing, assuming the abstraction holds up and everything available via the base OpenAIClient fully works when the concrete type is AzureOpenAIClient. @trrwilson, I assume that's the goal and anything that deviated from that would be a bug to be fixed?

Right, with the exception of non-overlapping capabilities; e.g. you won't be able to get/use a ModerationClient from AzureOpenAIClient because Azure OpenAI doesn't have a standalone /moderations API.

Thanks. It looks like these are the only two OpenAIClient methods today that will throw: https://github.com/Azure/azure-sdk-for-net/blob/aec1a1389636a2ef76270ab4bdcb0715a2abb1aa/sdk/openai/Azure.AI.OpenAI/src/Custom/AzureOpenAIClient.cs#L230-L242 Are there other not-supported things on individual clients, or the expectation is if you can get the relevant client everything "just works"?

trrwilson commented 4 months ago

Thanks. It looks like these are the only two OpenAIClient methods today that will throw: https://github.com/Azure/azure-sdk-for-net/blob/aec1a1389636a2ef76270ab4bdcb0715a2abb1aa/sdk/openai/Azure.AI.OpenAI/src/Custom/AzureOpenAIClient.cs#L230-L242 Are there other not-supported things on individual clients, or the expectation is if you can get the relevant client everything "just works"?

There are a small number of errata, but overwhelmingly it's that expectation: if you can get the scenario client, you should then be able to interact with that scenario client (inputs/outputs) the same way -- without any undue consideration of whether it came from OpenAI v1 or an Azure OpenAI endpoint.

bradygaster commented 4 months ago

I'd very-much like to have at-least an option to use the .NET OpenAI client. It seems like the natural option for .NET customers and not having it in Aspire is somewhat unfortunate.

Ideally, I could just pass an instance of either the Azure OpenAI Client or the .NET OpenAI client into a method/constructor and "it just work." Seems like, if they're that close to structure, we could get by putting them both in the package and giving the, both to customers.

I know it's harder than that and stuff, but, that'd be an ideal middle-of-the-road approach for folks in both cohorts.

tg-msft commented 3 months ago

One approach Aspire could take w.r.t. to DI is:

  • Aspire.OpenAI adds

    • an OpenAIClient service to DI
  • Aspire.Azure.AI.OpenAI adds

    • an AzureOpenAIClient service to DI
    • and the same object as an OpenAIClient service

That way someone just looking for an OpenAIClient can bind to either, and people can switch between Azure and non-Azure, if they want. And if someone is specifically looking for an AzureOpenAIClient, it will only work when using Aspire.Azure.AI.OpenAI.

I love the idea of injecting both the base and derived classes for the Azure scenario. Do we need two separate Aspire components for it though? We could potentially have a single Aspire.OpenAI component that would use the endpoint to construct either client, always inject an OpenAIClient, and sometimes inject an AzureOpenAIClient when relevant. Then the 95% use case focused on the base OpenAIClient will work regardless of where it's hosted. Customers could update their AppHost to change between OAI and AOAI without touching their other projects - just like they already do for things like running Postgres in a container vs. running Postgres hosted on Azure. The only downsides of that approach are the additional Azure dependencies including Azure.AI.OpenAI, Azure.Core, and Azure.Identity for people using just OAI. We've already got Azure support in the official Python library though and structured our .NET story to aim the same customer benefits. +@joseharriaga @trrwilson @KrzysztofCwalina @mayurid for their thoughts on how we'd want to position this.

stephentoub commented 3 months ago

Do we need two separate Aspire components for it though?

It seems strange to me that we'd do all this work to separate out the Azure.AI.OpenAI and OpenAI libraries, only to then effectively merge them back together again by having the Aspire.OpenAI layer incorporate both...?

eerhardt commented 3 months ago

Do we need two separate Aspire components for it though?

The only downsides of that approach are the additional Azure dependencies including Azure.AI.OpenAI, Azure.Core, and Azure.Identity for people using just OAI.

  1. I think the dependency concern is very important - why bring along Azure dependencies in an app that doesn't use Azure? This will open you up to security alerts when things like Azure.Identity report a security vulnerability
  2. It is confusing to understand/document when AzureOpenAIClient is injected into the DI container. My understanding of the proposal is that it will depend on configuration of the app what services are injected into the DI container, which I don't think is common practice. I'm not sure the error experience will be acceptable when an app really requires an AzureOpenAIClient, but only OAI was configured.

Maybe a hybrid approach would be to still have 2 Aspire components, but the Azure.AI.OpenAI component allowed being configured for an OAI endpoint, and only injected an OAIClient. My concern (2) above would still be there, but the dependency concerns would go away. And for people who wanted to allow either OAI or AOAI could just use the Azure.AI.OpenAI, which I think solves your concern.

joperezr commented 2 months ago

Might need to push this to next release as we had to prioritize testing work.

adventurehi commented 2 months ago

The documentation for the Aspire Azure AI OpenAI component indicates that it works with both OpenAI and Azure OpenAI. It did work prior to Aspire.Azure.AI.OpenAI 8.1.0-preview.1.24373.2 but now it doesn't work and throws an exception in here

                if (settings.Endpoint is null)
                {
                    throw new InvalidOperationException($"An OpenAIClient could not be configured. Ensure valid connection information was provided in 'ConnectionStrings:{connectionName}' or specify a '{nameof(AzureOpenAISettings.Endpoint)}' or '{nameof(AzureOpenAISettings.Key)}' in the '{configurationSectionName}' configuration section.");
                }
                else
                {

The exception says to setup the connection string with an endpoint or a key. The code is forcing an endpoint to be defined though which only works for Azure.

I was injecting OpenAIClient into the constructor, but with the current version i am doing it similar to the following.

   private readonly OpenAIClient _aiClient;
   private readonly ChatClient _chatClient;

    public ChatCompletionService()
    {
        var connection = Environment.GetEnvironmentVariable("ConnectionStrings__openai")?.Replace("Key=", "");
         _aiClient = new(connection);
         var chatModel = Environment.GetEnvironmentVariable("AI__OPENAI__CHATMODEL");
          _chatClient = _aiClient.GetChatClient(chatModel);
    }

Is non-Azure OpenAI supposed to still work with Aspire.Azure.AI.OpenAI 8.1.0-preview.1.24373.2? Is there a better way to handle things?

I think that It would be nice to have separate components for both.

mitchdenny commented 2 months ago

/cc @eerhardt @tg-msft

eerhardt commented 2 months ago

Is non-Azure OpenAI supposed to still work with Aspire.Azure.AI.OpenAI 8.1.0-preview.1.24373.2? Is there a better way to handle things?

This issue is intended to solve these issues.