Skip to content

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tarekgh
Copy link
Member

@tarekgh tarekgh commented May 27, 2025

Fixes #36123

@Copilot Copilot AI review requested due to automatic review settings May 27, 2025 19:42
Copy link
Contributor

@Copilot Copilot AI left a 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.

Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

@tarekgh
Copy link
Member Author

tarekgh commented May 27, 2025

@ericstj @davidfowl could you please have a quick look? Thanks!

}
else if (key.StartsWith(ApiHubPrefix, StringComparison.OrdinalIgnoreCase))
{
HandleMatchedConnectionStringPrefix(data, ApiHubPrefix, null, key, value);
Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

@ericstj ericstj May 27, 2025

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.

Copy link
Member

@ericstj ericstj left a 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EnvironmentVariablesConfigurationProvider should support all types of connectionstrings
2 participants