Add Azure AI OpenAI component #1475
Conversation
dotnet-issue-labeler
bot
added
the
area-components
|
How will this play if someone wants to use OpenAI not Azure OpenAI Service? Or would that be a different package that you'd use since there's no "deployment" component to using OpenAI? |
Right now this works as you can pass the api endpoint with the platform key from OpenAI in the connection string, and since Assuming we need a different resource type because of deployments, I think we should still have a single package containing a different resource and extensions. The Aspire component is mapped to the NuGet package after all. |
Looking at https://github.com/dotnet/aspire/pull/1475/files#diff-6167a4d1debba09c2aa1edf9ddd056d0b186c211f8a7078d69aee859abad774cR71-R74 is seems like the |
I thought one could just pass the ServiceUri and as I looked again at the sample to show you I realized it was not taking a Whatever, I will fix that with the new resource type that is required. |
|
Ah yes, I see the confusion, the Azure.AI.OpenAI doc is confusing as OpenAI takes a single value, a string of random letters/symbols/numbers. |
|
I could not find an Azure.ResourceManager package for OpenAI (I assume this was hinted before). Should I implement a provisioner by generating arm/bicep or wait for an official package? |
src/Components/Aspire.Azure.AI.OpenAI/AspireOpenAIExtensions.cs
Outdated
Show resolved
Hide resolved
src/Components/Aspire.Azure.AI.OpenAI/AspireOpenAIExtensions.cs
Outdated
Show resolved
Hide resolved
You should be able to use the Azure.ResourceManager API directly and just pass in Bicep for the local provisioner. |
|
The secrets PR was merged in. we now have a SecretResource type that you can use for Open AI. |
src/Components/Aspire.Azure.AI.OpenAI/AspireAzureOpenAIExtensions.cs
Outdated
Show resolved
Hide resolved
tests/Aspire.Azure.AI.OpenAI.Tests/AspireAzureAIOpenAIExtensionsTests.cs
Outdated
Show resolved
Hide resolved
…ons.cs Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
| @@ -11,20 +11,25 @@ namespace Aspire.Hosting.ApplicationModel; | |||
| /// <param name="name">The name of the resource.</param> | |||
| public class AzureOpenAIResource(string name) : Resource(name), IAzureResource, IResourceWithConnectionString | |||
| { | |||
| private readonly Collection<AzureOpenDeploymentResource> _deployments = []; | |||
| private readonly Collection<AzureOpenAIDeploymentResource> _deployments = []; | |||
There was a problem hiding this comment.
Collection<T>? Why not List<T>
There was a problem hiding this comment.
@davidfowl I followed the way AzureSqlServerResource was implemented since it had a similar model. Should I go ahead and replace this by IList/IReadOnlyList in both resources?
| /// <summary> | ||
| /// Represents an Azure OpenAI Deployment resource. | ||
| /// </summary> | ||
| public class AzureOpenAIDeploymentResource : Resource, |
There was a problem hiding this comment.
If we are defining a connection string format, perhaps we should include a connection string that includes the deployment name too?
There was a problem hiding this comment.
.. no need to block on this feedback though.
| /// <param name="builder">The <see cref="IDistributedApplicationBuilder"/>.</param> | ||
| /// <param name="name">The name of the resource. This name will be used as the connection string name when referenced in a dependency.</param> | ||
| /// <returns>A reference to the <see cref="IResourceBuilder{OpenAIResource}"/>.</returns> | ||
| public static IResourceBuilder<OpenAIResource> AddOpenAI(this IDistributedApplicationBuilder builder, string name) |
There was a problem hiding this comment.
This doesn't make sense to me. Let's say I'm building an app that uses Open AI (not Azure Open AI) and I want to deploy it on ACA via AZD. This won't work because the manifest will contain an openai.v0 resource which AZD doesn't know how to handle.
What we'll probably need to do here is have this resource type emit a connectionString field which contains the Open AI connection string. But for security reasons it would reference a secretstore/secret which in turn would use an input field to capture this detail during deployment.
We can get this in to main and I can help you work through this though since the secrets stuff is pretty new and not baked into AZD (yet).
There was a problem hiding this comment.
This won't work because the manifest will contain an openai.v0 resource which AZD doesn't know how to handle.
I thought the idea of the manifest was to describe all it could, and the consumers to convert to what they can use. In this case would do nothing for openai.v0.
Should I remove this line from the manifest at least?
…4555) Having an OpenAIClient in DI will become increasingly common with dotnet/aspire#1475. cc: @sebastienros
…4555) Having an OpenAIClient in DI will become increasingly common with dotnet/aspire#1475. cc: @sebastienros
…4555) Having an OpenAIClient in DI will become increasingly common with dotnet/aspire#1475. cc: @sebastienros
…4555) Having an OpenAIClient in DI will become increasingly common with dotnet/aspire#1475. cc: @sebastienros
Microsoft Reviewers: Open in CodeFlow