Add OpenAI component #5621

sebastienros · 2024-09-09T19:41:01Z

Contributes to #4567

Microsoft Reviewers: Open in CodeFlow

# Conflicts: # Aspire.sln

# Conflicts: # Aspire.sln # src/Components/Aspire.Azure.AI.OpenAI/ConfigurationSchema.json

src/Components/Aspire.OpenAI/Aspire.OpenAI.csproj

tests/Aspire.OpenAI.Tests/Aspire.OpenAI.Tests.csproj

eerhardt · 2024-09-16T22:27:16Z

Is there also work to enable Aspire.Azure.AI.OpenAI to work with just an OpenAIClient when configured to talk to OpenAI or a local emulator like Ollama? Or do you see that as a separate PR?

src/Components/Telemetry.md

src/Components/Aspire.OpenAI/README.md

eerhardt · 2024-09-17T14:29:10Z

src/Components/Aspire.OpenAI/README.md

+}
+```
+
+To learn how to use the `OpenAIClient` class refer to [Using the OpenAIClient class](https://github.com/openai/openai-dotnet?tab=readme-ov-file#using-the-openaiclient-class).


How permanent is this link? Is there an official doc site we can link to instead?

~~Just found a new page in docs, updating.~~ Nope, couldn't find anything more official.

I believe @AngelosP would be the best person to share what are considered official docs for this library.

src/Components/Aspire.OpenAI/README.md

src/Components/Aspire.OpenAI/AssemblyInfo.cs

src/Components/Aspire.OpenAI/README.md

src/Components/Aspire.OpenAI/AspireOpenAIExtensions.cs

src/Components/Aspire.OpenAI/AssemblyInfo.cs

tests/Aspire.OpenAI.Tests/ConformanceTests.cs

annelo-msft · 2024-09-17T21:13:54Z

src/Components/Aspire.OpenAI/OpenAISettings.cs

+/// <summary>
+/// The settings relevant to accessing OpenAI.
+/// </summary>
+public sealed class OpenAISettings


@KrzysztofCwalina, any concerns about adding this as a public type in this namespace? I know you and I had discussed potentially putting such types in the SCM-based client namespaces themselves.

Or, @eerhardt, could you help us understand established precedents around these settings types for Aspire more broadly? If we e.g. wanted to enable injection of an OpenAI client in a DI container outside the context of Aspire, would we want this type to live in a non-Aspire namespace?

These settings types are used in Aspire to drive what and how services are registered in DI. This is standard in Aspire, even components have it.

If we wanted to also add a type called OpenAISettings in the OpenAI client namespace, would that be frowned upon?

It wouldn't be great, but it would be possible. The namespace can disambiguate.

For Aspire, the pattern is {NameOfLibrary}Settings in the Aspire.{NameOfLibrary} namespace.

See

aspire/src/Components/Aspire.RabbitMQ.Client/RabbitMQClientSettings.cs

Lines 6 to 9 in 0490bcb

/// <summary>

/// Provides the client configuration settings for connecting to a RabbitMQ message broker.

/// </summary>

public sealed class RabbitMQClientSettings

aspire/src/Components/Aspire.StackExchange.Redis/StackExchangeRedisSettings.cs

Lines 6 to 9 in 0490bcb

/// <summary>

/// Provides the client configuration settings for connecting to a Redis server.

/// </summary>

public sealed class StackExchangeRedisSettings

Cool, as long as we can keep that an an option in our back pocket, I have no concerns.

src/Components/Aspire.OpenAI/README.md

annelo-msft · 2024-09-17T21:26:05Z

src/Components/Aspire.OpenAI/README.md

+
+## AppHost extensions
+
+There is no specific AppHost extension corresponding to the OpenAI integration. Instead a connection string resource can be registered:


For my learning -- what is an AppHost extension and is this something we would want to have a story for in the .NET ecosystem to support better ease of use for OpenAI? Mostly just curious on this piece.

An AppHost extension is a package that helps provision or describe the server part of the integration. For instance with Azure.AI.OpenAI there is a server resource that can be registered, will AI models too, such that we can provision the resource when the app is deployed. With OpenAI there is no such thing we can deploy, so we only declare a connection string, like a logical representation of this resource. Ultimately we could have a component that provides Ollama services for example.

Thanks! My comment here doesn't block this PR in any way, but I'm interested in understanding why we have one for Azure.AI.OpenAI and not OpenAI? If there are pieces needed uniquely for SCM-based clients I'd be interested to understand the gaps there and if pieces are needed from SCM to address those gaps?

why we have one for Azure.AI.OpenAI and not OpenAI?

Because you can provision an Azure OpenAI service (using bicep). That's what the "AppHost" extension will do.

You can't provision an OpenAI service. You need to sign up on their website.

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

# Conflicts: # src/Components/Aspire.OpenAI/README.md

src/Components/Aspire.OpenAI/AssemblyInfo.cs

src/Components/Aspire.OpenAI/OpenAISettings.cs

eerhardt · 2024-09-19T19:00:44Z

src/Components/Aspire.OpenAI/OpenAISettings.cs

+/// <summary>
+/// The settings relevant to accessing OpenAI.
+/// </summary>
+public sealed class OpenAISettings


It wouldn't be great, but it would be possible. The namespace can disambiguate.

For Aspire, the pattern is {NameOfLibrary}Settings in the Aspire.{NameOfLibrary} namespace.

See

aspire/src/Components/Aspire.RabbitMQ.Client/RabbitMQClientSettings.cs

Lines 6 to 9 in 0490bcb

/// <summary>

/// Provides the client configuration settings for connecting to a RabbitMQ message broker.

/// </summary>

public sealed class RabbitMQClientSettings

aspire/src/Components/Aspire.StackExchange.Redis/StackExchangeRedisSettings.cs

Lines 6 to 9 in 0490bcb

/// <summary>

/// Provides the client configuration settings for connecting to a Redis server.

/// </summary>

public sealed class StackExchangeRedisSettings

eerhardt · 2024-09-19T19:04:15Z

src/Components/Aspire.OpenAI/Aspire.OpenAI.csproj

+    <SuppressFinalPackageVersion>true</SuppressFinalPackageVersion>
+    <PackageTags>ai openai</PackageTags>
+    <Description>A client for OpenAI that integrates with Aspire, including metrics, logging and telemetry.</Description>
+    <PackageIconFullPath>$(SharedDir)OpenAI_256x.png</PackageIconFullPath>


@DamianEdwards - you've handled this for other integrations. Can you help here?

src/Components/Aspire.OpenAI/Aspire.OpenAI.csproj

src/Components/Aspire.OpenAI/AspireOpenAIExtensions.cs

sebastienros · 2024-09-20T17:52:45Z

Failing on unrelated cosmos tests

eerhardt

LGTM.

Directory.Packages.props

eerhardt · 2024-09-23T19:55:48Z

tests/Aspire.OpenAI.Tests/AspireOpenAIExtensionsTests.cs

+            host.Services.GetRequiredKeyedService<OpenAIClient>("openai") :
+            host.Services.GetRequiredService<OpenAIClient>();
+
+        Assert.NotNull(client);


Can we assert on something more than "not null"?

I meant, can we assert something like the connection string or URI is correct?

That was my initial thought too but I couldn't find it in the model, do you know if it's somewhere? I didn't dig super deep, maybe I missed it. Will check again.

Are you fine with private reflection in this case?

Nah, let's just live with what we have. If we mis-configured it, we will know because things won't work.

sebastienros · 2024-09-24T00:39:00Z

/azp run

azure-pipelines · 2024-09-24T00:39:12Z

Azure Pipelines successfully started running 1 pipeline(s).

sebastienros added 6 commits August 20, 2024 08:54

Create OpenAI component project files

613f117

Merge remote-tracking branch 'origin/main' into sebros/openai

4742464

Keep Endpoint

314e86b

Update README

8726de7

Add tests

fe7333c

Merge remote-tracking branch 'origin/main' into sebros/openai

d9d89fa

# Conflicts: # Aspire.sln

sebastienros requested review from radical and eerhardt as code owners September 9, 2024 19:41

dotnet-issue-labeler bot added the area-components label Sep 9, 2024

sebastienros added 4 commits September 9, 2024 12:43

Update package version

9662840

Clean project files

c645e7e

Enable tracing tests

f3d62bc

Fix component name

ccd4a95

davidfowl added area-integrations Issues pertaining to Aspire Integrations packages and removed area-components labels Sep 10, 2024

sebastienros added 3 commits September 16, 2024 09:11

Update Azure.AI.OpenAI schema

1d31619

Merge remote-tracking branch 'origin/main' into sebros/openai

d18e13e

# Conflicts: # Aspire.sln # src/Components/Aspire.Azure.AI.OpenAI/ConfigurationSchema.json

Merge branch 'main' into sebros/openai

daa9922

build-analysis bot mentioned this pull request Sep 16, 2024

Some tests timing out on helix #5247

Open

radical reviewed Sep 16, 2024

View reviewed changes

src/Components/Aspire.OpenAI/Aspire.OpenAI.csproj Outdated Show resolved Hide resolved

radical reviewed Sep 16, 2024

View reviewed changes

tests/Aspire.OpenAI.Tests/Aspire.OpenAI.Tests.csproj Outdated Show resolved Hide resolved

Fix TFM variables

a6fd23b

eerhardt reviewed Sep 17, 2024

View reviewed changes

src/Components/Telemetry.md Show resolved Hide resolved

eerhardt reviewed Sep 17, 2024

View reviewed changes

sebastienros mentioned this pull request Sep 17, 2024

AddOpenAIRestApiClient to wrap both AzureOpenAI and OpenAI integrations #5755

Closed

annelo-msft reviewed Sep 17, 2024

View reviewed changes

src/Components/Aspire.OpenAI/README.md Outdated Show resolved Hide resolved

annelo-msft reviewed Sep 17, 2024

View reviewed changes

Update src/Components/Aspire.OpenAI/README.md

3fb8bd1

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

sebastienros and others added 3 commits September 17, 2024 15:12

Update src/Components/Aspire.OpenAI/README.md

7a778e4

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

PR feedback

82844da

Merge remote-tracking branch 'origin/sebros/openai' into sebros/openai

a10a215

# Conflicts: # src/Components/Aspire.OpenAI/README.md

eerhardt reviewed Sep 19, 2024

View reviewed changes

sebastienros mentioned this pull request Sep 19, 2024

Add method to select the OpenAI component based on configuration #5789

Merged

sebastienros added 2 commits September 19, 2024 16:45

Apply feedback

f188cdd

Remove logo

c901ba0

build-analysis bot mentioned this pull request Sep 20, 2024

The active test run was aborted. Reason: Test host process crashed dotnet/dnceng#451

Open

3 tasks

eerhardt approved these changes Sep 23, 2024

View reviewed changes

sebastienros added 2 commits September 23, 2024 16:05

Update package versions

16a3c37

Add type assert

de93509

sebastienros enabled auto-merge (squash) September 24, 2024 01:14

sebastienros merged commit b486f32 into main Sep 24, 2024
11 checks passed

sebastienros deleted the sebros/openai branch September 24, 2024 01:49

sebastienros added this to the 9.0 milestone Sep 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add OpenAI component #5621

Add OpenAI component #5621

sebastienros commented Sep 9, 2024 •

edited

Loading

eerhardt commented Sep 16, 2024

eerhardt Sep 17, 2024

sebastienros Sep 17, 2024 •

edited

Loading

annelo-msft Sep 18, 2024

annelo-msft Sep 17, 2024

sebastienros Sep 17, 2024

annelo-msft Sep 18, 2024

eerhardt Sep 19, 2024

annelo-msft Sep 19, 2024

annelo-msft Sep 17, 2024

sebastienros Sep 17, 2024

annelo-msft Sep 19, 2024

eerhardt Sep 19, 2024

eerhardt Sep 19, 2024

eerhardt Sep 19, 2024

sebastienros commented Sep 20, 2024

eerhardt left a comment

eerhardt Sep 23, 2024

eerhardt Sep 23, 2024

sebastienros Sep 23, 2024

sebastienros Sep 23, 2024

eerhardt Sep 23, 2024

sebastienros commented Sep 24, 2024

azure-pipelines bot commented Sep 24, 2024

	/// <summary>
	/// Provides the client configuration settings for connecting to a RabbitMQ message broker.
	/// </summary>
	public sealed class RabbitMQClientSettings


		## AppHost extensions

		There is no specific AppHost extension corresponding to the OpenAI integration. Instead a connection string resource can be registered:

Add OpenAI component #5621

Add OpenAI component #5621

Conversation

sebastienros commented Sep 9, 2024 • edited Loading

Microsoft Reviewers: Open in CodeFlow

eerhardt commented Sep 16, 2024

Choose a reason for hiding this comment

sebastienros Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastienros commented Sep 20, 2024

eerhardt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastienros commented Sep 24, 2024

azure-pipelines bot commented Sep 24, 2024

sebastienros commented Sep 9, 2024 •

edited

Loading

sebastienros Sep 17, 2024 •

edited

Loading