-
Notifications
You must be signed in to change notification settings - Fork 738
Enhance resource management for connection strings #7826
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
Introduces the `IResourceWithoutLifetime` interface for resources without a lifetime, such as parameters and connection strings. Updates `ParameterResource` to implement this interface and adds the `ConnectionStringParameterResource` class for better management of connection string parameters. Implements the `AddConnectionString` method in `ConnectionStringBuilderExtensions` and updates `ApplicationOrchestrator` to handle resources without a lifetime correctly. Tests have been added and modified to verify the new functionality, and `Program.cs` has been updated to demonstrate the new connection string capabilities, including the addition of a second connection string with a parameter value. These changes improve the overall resource management in the application, particularly for connection strings and parameters.
|
@JamesNK and @adamint, I'd like to follow up on this work with a way to show these resources without a lifetime in the dashboard. Right now, we hide parameters and connection strings in the dashboard but that isn't ideal. There's also no way to unhide resources. I'd like to solve a couple of things here:
|
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.
PR Overview
This pull request enhances resource management by introducing the IResourceWithoutLifetime interface and a dedicated ConnectionStringParameterResource to improve handling of connection strings and parameter resources. Key changes include:
- Adding IResourceWithoutLifetime and updating ParameterResource to implement it.
- Introducing ConnectionStringParameterResource and updating related extension methods.
- Extending orchestrator logic and tests to support resources without a lifetime.
Reviewed Changes
| File | Description |
|---|---|
| src/Aspire.Hosting/ConnectionStringBuilderExtensions.cs | Added extension methods for connection string resources with updated XML comments. |
| src/Aspire.Hosting/ApplicationModel/IResourceWithoutLifetime.cs | Introduced new interface for resources without a lifetime. |
| src/Aspire.Hosting/ConnectionStringResource.cs | Created a new resource class for connection strings implementing the new interface. |
| tests/Aspire.Hosting.Tests/* | Added/updated tests to verify connection string and parameter resource functionality. |
| src/Aspire.Hosting/ParameterResource.cs | Updated ParameterResource to implement IResourceWithoutLifetime and added a ConfigurationKey property. |
| src/Aspire.Hosting/ParameterResourceBuilderExtensions.cs | Updated resource builder extensions to use the new ConnectionStringParameterResource. |
| src/Aspire.Hosting/ApplicationModel/ExpressionResolver.cs | Adjusted pattern matching to work with new resource types. |
| src/Aspire.Hosting/Orchestrator/ApplicationOrchestrator.cs | Added processing logic for resources without a lifetime in orchestrator. |
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
src/Aspire.Hosting/ApplicationModel/ResourceNotificationService.cs
Outdated
Show resolved
Hide resolved
| internal string ConfigurationKey | ||
| { | ||
| get => _configurationKey ?? (IsConnectionString ? $"ConnectionStrings:{Name}" : $"Parameters:{Name}"); | ||
| set => _configurationKey = value; | ||
| } |
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.
Glad this is internal. I want to avoid strictly tying the concept of a parameter/connection string to the .NET configuration store. They are connected for convienence however.
mitchdenny
left a comment
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.
A few suggestions for future enhancements and test improvements, but overall this looks good and seems to work with the run throughs that I made. I can see how this will be useful for rapidly creating wrapper APIs where the input is primarily something like an API key with a few extra bits and pieces bolted on.
| Assert.Equal("Endpoint=http://localhost:3452;Key=secretKey", config["ConnectionStrings__cs"]); | ||
| } | ||
|
|
||
| [Fact] |
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.
Add a test like we have in the ParameterEndToEnd playground where we use a resource (such as a SQL DB) as an input to the expression.
|
@JamesNK new dashboard failures? Are they related to your changes to tests? (https://github.com/dotnet/aspire/actions/runs/13622390377) |
Description
Introduces the
IResourceWithoutLifetimeinterface for resources without a lifetime, such as parameters and connection strings. UpdatesParameterResourceto implement this interface and adds theConnectionStringParameterResourceclass for better management of connection string parameters.Implements the
AddConnectionStringmethod inConnectionStringBuilderExtensionsand updatesApplicationOrchestratorto handle resources without a lifetime correctly.Tests have been added and modified to verify the new functionality, and
Program.cshas been updated to demonstrate the new connection string capabilities, including the addition of a second connection string with a parameter value.These changes improve the overall resource management in the application, particularly for connection strings and parameters.
Fixes #7641
Fixes #7642
Fixes #2964
Checklist
<remarks />and<code />elements on your triple slash comments?breaking-changetemplate):doc-ideatemplate):