-
Couldn't load subscription status.
- Fork 713
WaitFor Milvus #5707
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
WaitFor Milvus #5707
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,15 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Data.Common; | ||
| using Aspire.Hosting.ApplicationModel; | ||
| using Aspire.Hosting.Milvus; | ||
| using Aspire.Hosting.Utils; | ||
| using Aspire.Milvus.Client; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
| using Microsoft.Extensions.Diagnostics.HealthChecks; | ||
| using Microsoft.Extensions.Logging; | ||
| using Milvus.Client; | ||
|
|
||
| namespace Aspire.Hosting; | ||
|
|
||
|
|
@@ -51,6 +57,43 @@ public static IResourceBuilder<MilvusServerResource> AddMilvus(this IDistributed | |
|
|
||
| var milvus = new MilvusServerResource(name, apiKeyParameter); | ||
|
|
||
| string? connectionString = null; | ||
| MilvusClient? milvusClient = null; | ||
|
|
||
| builder.Eventing.Subscribe<ConnectionStringAvailableEvent>(milvus, async (@event, ct) => | ||
| { | ||
| connectionString = await milvus.ConnectionStringExpression.GetValueAsync(ct).ConfigureAwait(false); | ||
| if (connectionString is null) | ||
| { | ||
| throw new DistributedApplicationException($"ConnectionStringAvailableEvent was published for the '{milvus.Name}' resource but the connection string was null."); | ||
| } | ||
| milvusClient = CreateMilvusClient(@event.Services, connectionString); | ||
| var lookup = builder.Resources.OfType<MilvusDatabaseResource>().ToDictionary(d => d.Name); | ||
| foreach (var databaseName in milvus.Databases) | ||
| { | ||
| if (!lookup.TryGetValue(databaseName.Key, out var databaseResource)) | ||
| { | ||
| throw new DistributedApplicationException($"Database resource '{databaseName}' under Milvus Server resource '{milvus.Name}' was not found in the model."); | ||
| } | ||
| var connectionStringAvailableEvent = new ConnectionStringAvailableEvent(databaseResource, @event.Services); | ||
| await builder.Eventing.PublishAsync<ConnectionStringAvailableEvent>(connectionStringAvailableEvent, ct).ConfigureAwait(false); | ||
|
|
||
| var beforeResourceStartedEvent = new BeforeResourceStartedEvent(databaseResource, @event.Services); | ||
| await builder.Eventing.PublishAsync(beforeResourceStartedEvent, ct).ConfigureAwait(false); | ||
| } | ||
| }); | ||
|
|
||
| var healthCheckKey = $"{name}_check"; | ||
| // TODO: Use health check from AspNetCore.Diagnostics.HealthChecks once it's implemented via this issue: | ||
| // https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/issues/2214 | ||
| builder.Services.AddHealthChecks() | ||
| .Add(new HealthCheckRegistration( | ||
| healthCheckKey, | ||
| sp => new MilvusHealthCheck(milvusClient!), | ||
| failureStatus: default, | ||
| tags: default, | ||
| timeout: default)); | ||
|
|
||
| return builder.AddResource(milvus) | ||
| .WithImage(MilvusContainerImageTags.Image, MilvusContainerImageTags.Tag) | ||
| .WithImageRegistry(MilvusContainerImageTags.Registry) | ||
|
|
@@ -67,7 +110,8 @@ public static IResourceBuilder<MilvusServerResource> AddMilvus(this IDistributed | |
| { | ||
| ctx.EnvironmentVariables["COMMON_SECURITY_DEFAULTROOTPASSWORD"] = milvus.ApiKeyParameter; | ||
| }) | ||
| .WithArgs("milvus", "run", "standalone"); | ||
| .WithArgs("milvus", "run", "standalone") | ||
| .WithHealthCheck(healthCheckKey); | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -101,8 +145,33 @@ public static IResourceBuilder<MilvusDatabaseResource> AddDatabase(this IResourc | |
| databaseName ??= name; | ||
|
|
||
| builder.Resource.AddDatabase(name, databaseName); | ||
| var milvusResource = new MilvusDatabaseResource(name, databaseName, builder.Resource); | ||
| return builder.ApplicationBuilder.AddResource(milvusResource); | ||
| var milvusDatabaseResource = new MilvusDatabaseResource(name, databaseName, builder.Resource); | ||
|
|
||
| string? connectionString = null; | ||
| MilvusClient? milvusClient = null; | ||
| builder.ApplicationBuilder.Eventing.Subscribe<ConnectionStringAvailableEvent>(milvusDatabaseResource, async (@event, ct) => | ||
| { | ||
| connectionString = await milvusDatabaseResource.ConnectionStringExpression.GetValueAsync(ct).ConfigureAwait(false); | ||
| if (connectionString == null) | ||
| { | ||
| throw new DistributedApplicationException($"ConnectionStringAvailableEvent was published for the '{milvusDatabaseResource.Name}' resource but the connection string was null."); | ||
| } | ||
| milvusClient = CreateMilvusClient(@event.Services, connectionString); | ||
| }); | ||
|
|
||
| var healthCheckKey = $"{name}_check"; | ||
| // TODO: Use health check from AspNetCore.Diagnostics.HealthChecks once it's implemented via this issue: | ||
| // https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/issues/2214 | ||
| builder.ApplicationBuilder.Services.AddHealthChecks() | ||
| .Add(new HealthCheckRegistration( | ||
| healthCheckKey, | ||
| sp => new MilvusHealthCheck(milvusClient!), | ||
| failureStatus: default, | ||
| tags: default, | ||
| timeout: default)); | ||
|
|
||
| return builder.ApplicationBuilder.AddResource(milvusDatabaseResource) | ||
| .WithHealthCheck(healthCheckKey); | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -190,4 +259,44 @@ private static void ConfigureAttuContainer(EnvironmentCallbackContext context, M | |
| // This will need to be refactored once updated service discovery APIs are available | ||
| context.EnvironmentVariables.Add("MILVUS_URL", $"{resource.PrimaryEndpoint.Scheme}://{resource.Name}:{resource.PrimaryEndpoint.TargetPort}"); | ||
| } | ||
| internal static MilvusClient CreateMilvusClient(IServiceProvider sp, string? connectionString) | ||
| { | ||
| if (connectionString is null) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with @davidfowl we should do this because the created Milvus connection string in app-host isn't compatible with MilvusClient and those properties (database, endpoint, key) should be extracted before creating MilvusClient. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was there a reason this wasn't done? |
||
| { | ||
| throw new InvalidOperationException("Connection string is unavailable"); | ||
| } | ||
|
|
||
| Uri? endpoint = null; | ||
| string? key = null; | ||
| string? database = null; | ||
|
|
||
| if (Uri.TryCreate(connectionString, UriKind.Absolute, out var uri)) | ||
| { | ||
| endpoint = uri; | ||
| } | ||
| else | ||
| { | ||
| var connectionBuilder = new DbConnectionStringBuilder | ||
| { | ||
| ConnectionString = connectionString | ||
| }; | ||
|
|
||
| if (connectionBuilder.ContainsKey("Endpoint") && Uri.TryCreate(connectionBuilder["Endpoint"].ToString(), UriKind.Absolute, out var serviceUri)) | ||
| { | ||
| endpoint = serviceUri; | ||
| } | ||
|
|
||
| if (connectionBuilder.ContainsKey("Key")) | ||
| { | ||
| key = connectionBuilder["Key"].ToString(); | ||
| } | ||
|
|
||
| if (connectionBuilder.ContainsKey("Database")) | ||
| { | ||
| database = connectionBuilder["Database"].ToString(); | ||
| } | ||
| } | ||
|
|
||
| return new MilvusClient(endpoint!, apiKey: key!, database: database, loggerFactory: sp.GetRequiredService<ILoggerFactory>()); | ||
| } | ||
| } | ||
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.
Probably technically don't need the connection string here since you have the client.