-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add ConfigurationManager #55338
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
Add ConfigurationManager #55338
Conversation
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
|
Tagging subscribers to this area: @maryamariyan, @safern Issue DetailsThis adds a type already used by ASP.NET Core's new WebApplcation and WebApplicationBuilder that allows reading from configuration (e.g. New APInamespace Microsoft.Extensions.Configuration
{
+ public sealed class Config : IConfigurationRoot, IConfigurationBuilder, IDisposable
+ {
+ public Configuration();
+ public string? this[string key] { get; set; }
+ public IConfigurationSection GetSection(string key);
+ public void Dispose();
+ }The members that are required to implement IConfigurationBuilder will be implemented explicitly, so members like Usage Example
using var config = new Config();
config.AddEnvironmentVariables(prefix: "MyCustomPrefix_");
if (config["FileConfig"] == "enabled")
{
config.AddJsonFile("MyConfig.json", optional: true, reloadOnChange: true);
}
string myValueFromJson = config["JsonConfigValue"];
// ...We have docs where we demonstrate manually building an IConfigurationBuilder, reading from the built IConfiguration, and then throwing that away to add a new config source. This is an alternative to that. Fixes #51770
|
|
@pakrym @davidfowl Does this look good to you? |
|
Did we get a consensus on the name yet? As it was stated on the issue I don't think |
|
@safern We're clearing up the naming but can you review the PR? |
src/libraries/Microsoft.Extensions.Configuration/tests/ConfigTests.cs
Outdated
Show resolved
Hide resolved
| var memVal2 = config["Mem2:KeyInMem2"]; | ||
| var memVal3 = config["MEM3:KEYINMEM3"]; | ||
|
|
||
| // Assert |
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.
Nit: This comment doesn't seem that useful.
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 generally agree about the uselessness of // Arrange // Act // Assert comments, but this was copied from ConfigurationTest. If we change it, I'd rather do it in one pass.
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.
Ok, I don't mind if we do that cleanup in a separate PR.
safern
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.
LGTM
This adds a type already used by ASP.NET Core's new WebApplcation and WebApplicationBuilder that allows reading from configuration (e.g.
appsettings.jsonandDOTNET_/ASPNETCORE_environment variables) while still being able to add new configuration sources without explicitly rebuilding config. Every time a source is added via theIConfigurationBuilderinterface, theIConfigurationupdates automatically and immediately.New API
namespace Microsoft.Extensions.Configuration { + public sealed class ConfigurationManager : IConfigurationRoot, IConfigurationBuilder, IDisposable + { + public ConfigurationManager(); + public string? this[string key] { get; set; } + public IConfigurationSection GetSection(string key); + public void Dispose(); + }The members that are required to implement IConfigurationBuilder will be implemented explicitly, so members like
IList<IConfigurationSource> IConfigurationBuilder.Sourcesdon't pollute intellisense. Extension methods are generally used to add configuration sources.Usage Example
WebApplicationBuilderessentially already exposes this type as a public property. The problem it is trying to solve is being able to read config fromappsettings.jsonandDOTNET_/ASPNETCORE_while configuring the host'sIServiceCollectionwhile at the same time being able to add new configuration sources or even change the content root without having to introduce additional build stages.We have docs where we demonstrate manually building an IConfigurationBuilder, reading from the built IConfiguration, and then throwing that away to add a new config source. This is an alternative to that.
Fixes #51770