-
Notifications
You must be signed in to change notification settings - Fork 5k
Add missing connection strings to EnvironmentVariablesConfigurationProvider #116037
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
base: main
Are you sure you want to change the base?
Add missing connection strings to EnvironmentVariablesConfigurationProvider #116037
Conversation
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.
Pull Request Overview
This PR adds missing connection string support for additional services and databases in the EnvironmentVariablesConfigurationProvider.
- Added support for PostgreSQL, API Hub, DocDB, Event Hub, Notification Hub, Redis Cache, and Service Bus connection strings.
- Introduced corresponding tests to verify the new connection string configurations.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/libraries/Microsoft.Extensions.Configuration.EnvironmentVariables/tests/EnvironmentVariablesTest.cs | Added tests to validate the loading and handling of new connection string types. |
src/libraries/Microsoft.Extensions.Configuration.EnvironmentVariables/src/EnvironmentVariablesConfigurationProvider.cs | Introduced new connection string prefixes and handling logic for additional services. |
Tagging subscribers to this area: @dotnet/area-extensions-configuration |
@ericstj @davidfowl could you please have a quick look? Thanks! |
} | ||
else if (key.StartsWith(ApiHubPrefix, StringComparison.OrdinalIgnoreCase)) | ||
{ | ||
HandleMatchedConnectionStringPrefix(data, ApiHubPrefix, null, key, 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.
I assume for all these that it's expected that we don't use a provider name?
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.
That's correct. Non-relational services—such as API Hub, Cosmos DB, Event Hubs, Notification Hubs, Redis Cache, Service Bus, and others—do not use traditional ADO.NET providers. The documentation at this link outlines which prefix is injected into a .NET app along with its corresponding provider. Additionally, in the related issue thread, it was noted that a workaround is to define a custom connection string that already uses a null
provider.
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.
Now that we're adding a 1st class support, does it help to set a provider name though? How does that get used by folks? If it is intentional that we don't set a provider name then it would be good to capture that reasoning in 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.
It looks reasonable to me - but I think we should get a second opinion from folks who know better how these are used to understand if any require provider names. My very cursory understanding here is that the provider name could help some generic component auto-discover a registered provider and use that to connect. Which seems like it could be an important thing to provide better built-in support.
Fixes #36123