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
API Proposal: Add BindConfiguration extension method for OptionsBuilder #36209
Comments
I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label. |
/cc: @davidfowl |
This is a good idea! I would rename sectionName to path (since you want to support A:B:C) cc @HaoK |
Sounds like a great idea to me as well! |
@davidfowl The Bind extension method on IConfiguration uses "key" as the paramter name. Either "key" or "path" seem good names to me?... |
I would be explicit and call it configPath. Also thinking about this more, it's not clear to me where you'd use this without the section |
@davidfowl What do you mean by that? Are you referring to the nullability of the argument? I agree that you almost never want to have a null configuration path, but since we'd implement that over the But I agree that it is most nonsensical to do so, I have no problems with making it a required parameter. |
Updated proposal above in reaction to #36209 (comment) and #36209 (comment).
|
namespace Microsoft.Extensions.DependencyInjection
{
public static class OptionsBuilderConfigurationExtensions
{
public static OptionsBuilder<TOptions> BindConfiguration<TOptions>(
this OptionsBuilder<TOptions> optionsBuilder,
string configSectionPath,
Action<BinderOptions> configureBinder = null)
where TOptions : class => null;
}
} |
@maryamariyan I have opened #39825 which implements this proposal. @maryamariyan, @davidfowl, while I was implementing the API, I noticed why I had originally proposed the |
Not sure if this is the appropriate place to ask but I'm uncertain if I found a bug or misunderstood something. In the original issue it was said
Until recently, I only used the generic host as part of ASP.NET and didn't have to think much about this. However, I just got started with a console application using Is this expected or a bug? If this isn't a short and obvious answer I can open a standalone issue for it but I thought I'd ask here first. To reproduce just create a new .NET 5 console application, add Microsoft.Extensions.Hosting and replace the contents of using System.Threading.Tasks;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
namespace Generic_Host_Configuration_Issue
{
public static class Program
{
public static Task Main(string[] args) => Host.CreateDefaultBuilder(args)
.ConfigureServices(ConfigureServices)
.RunConsoleAsync();
private static void ConfigureServices(HostBuilderContext hostBuilderContext, IServiceCollection services)
{
services.AddHostedService<SampleHostedService>();
// This works as expected and updates when the file is changed
services.Configure<SampleOptions>(hostBuilderContext.Configuration.GetSection("Sample"));
// This also works
//services.AddOptions<SampleOptions>()
// .Bind(hostBuilderContext.Configuration.GetSection("Sample"));
// But this doesn't
//services.AddOptions<SampleOptions>()
// .BindConfiguration("Sample");
}
}
} Add the following classes. public class SampleOptions
{
public string MyText { get; set; }
public int MyInt { get; set; }
} public class SampleHostedService : BackgroundService
{
private readonly ILogger<SampleHostedService> _logger;
private readonly IOptionsMonitor<SampleOptions> _optionsMonitor;
public SampleHostedService(ILogger<SampleHostedService> logger, IOptionsMonitor<SampleOptions> optionsMonitor)
{
_logger = logger;
_optionsMonitor = optionsMonitor;
}
protected override async Task ExecuteAsync(CancellationToken stoppingToken)
{
while (!stoppingToken.IsCancellationRequested)
{
var current = _optionsMonitor.CurrentValue;
_logger.LogInformation($"MyText: {current.MyText} | MyInt: {current.MyInt}");
await Task.Delay(3000, stoppingToken).ConfigureAwait(false);
}
}
} Now the only thing left is an {
"Sample": {
"MyText": "Hello World!",
"MyInt": 42
}
} Now you can run the application and make changes in the |
@Joelius300 hmmm, both your example Bind from Configuration on the HostBuilderContext. My extension actually binds from the fully materialized Configuration that also takes AddConfiguration calls within Startup into account. Can you test what happens if you call: |
@Joelius300 so what I am saying is that it is rather a bug in the Chained Configuration Source, not actually in this extension method. That being said, I don't know whether they wanted this behaviour in the chained configuration or whether it is a bug. Could you open up a new issue detailing this (and tag me, since I too would like to know 😂)? |
I know but does that prevent the reload-on-change functionality? That just doesn't seem inteded to me.
How would I call that? Didn't manage to find that function on
Maybe but I thought I'd ask here first 😅 |
@Joelius300 huh? Configure<TOptions, TDep1>() should d be on OptionsBuilder? 🤔 |
I can do this services.AddOptions<SampleOptions>()
.Configure<IConfiguration>((options, config) => config.Bind(options)); but that just gives me an empty option (all default values). |
@maryamariyan we should fix this one. |
@davidfowl a-ha! 😄 Should I open a separate issue to document it or do these comments here suffice? |
Hrm, so generally bind doesn't take part in registering for notifications, in the past this gets hooked up via a Configure, but you can always just directly add the change source for reload to work: |
Since basically change notifications is orthogonal, its needing that change token source that's hooked up to config.reload, and turning on reloadOnChanges on the config file itself |
Ah I see, this is named a BindConfiguration (but its really a Configure), so yeah that method should also register the change source, should be an easy fix |
Summary
BindConfiguration
extension method forOptionsBuilder<TOptions>
.Description
There are situations in which the
IConfiguration
instance from which to configure options has not yet been materialized when a method likeConfigureServices
is being executed. For example, the .NET Core Generic Host Builder offers theConfigureServices
method which does not offer access to the Application Configuration instance that will materialize when the Host is built. (Note: A fully materialized host configuration instance can be obtained from theHostBuilderContext
instance that is passed to the callback, however, this host configuration can differ from the Application configuration and may not offer the desired behaviour.)In such situations, the
Bind
extension methods on theOptionsBuilder<TOptions>
cannot be called, since they all require a fully materializedIConfiguration
instance. Instead, you need to call theConfigure
extension method, specifyingIConfiguration
as a DI dependency for configuration and then call theIConfiguration.Bind
extension method in the specifying configuration callback action.Since binding from a DI-provided
IConfiguration
is a fairly usual scenario when using the .NET Generic Host (i.e. without using a Startup class), it would be advantageous to simplify this scenario by providing 1st class support in form of an extension method that performs the necessary steps.Proposal
BindConfiguration
extension method forOptionsBuilder<TOptions>
.Usage
Previous implementation
The proposed API is a convenience wrapper around existing APIs. The code above could previously be implemented as follows:
Comments
The name
BindConfiguration
was chosen, since the arguments of the extension method do not directly conway how binding is performed. The existingBind
extension methods all take anIConfiguration
instance as argument, and in these cases the binding is implied from the argument type. Conceptually, the proposed API can be thought of as an overload to the existingBind
extension methods.Implementation
#34191 specifies a possible implementation for this API
The text was updated successfully, but these errors were encountered: