Add missing connection strings to EnvironmentVariablesConfigurationProvider#116037
Conversation
There was a problem hiding this comment.
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. |
|
@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.
I assume for all these that it's expected that we don't use a provider name?
There was a problem hiding this comment.
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.
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.
I created a tracking issue #116226 to follow up for provider names so we don't have to block this PR.
ericstj
left a comment
ericstj
left a comment
There was a problem hiding this comment.
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