Revisit OptionsMonitor/ReloadableOptions/ChangeTracking

[HaoK]

No description provided.

[arielcloud]

is there a deadline for this feature?
is there a way for now to reload options on configuration change?

[divega]

Assigning to 1.1.0 after talking to @glennc about @DamianEdwards feedback on this.

[MisterJames]

@HaoK I see that this is still needing design, but if this is something that can be worked on by the community I'd like to take a stab at this (or part of it).

[HaoK]

[IUpdateableOptions].Value (name tweaking TBD)
Scoped cache of IOptionsMonitor.CurrentValue

[divega]

We also decided that serviceCollection.Configure<MyOptions>(configuration) would hookup the necessary pieces to use IUpdateableOptions without requiring any flags (unlike our previous iteration of the feature).

[HaoK]

How about IOptionsState instead of IUpdateableOptions giving us:

IOptions [Singleton permanent value]
IOptionsMonitor [Singleton permanent until notifications to trigger rebuild]
IOptionsState [ Captures the options state from the monitor for the scope of the request]

cc @davidfowl @DamianEdwards @Eilon

[divega]

I prefer IOptionsSnapshot to IUpdatableOptions or IOptionsState.

cc @davidfowl @DamianEdwards @Eilon

[HaoK]

Snapshot works for me as well

[DamianEdwards]

Not feeling it. Need to stew on it some more...

[Eilon]

Boo.

[divega]

I think it is a pretty good name but besides, IOptionsSnapshot is the first name we have discussed that accurately reflects what it is and doesn't convey wrong expectations, e.g. you would not expect to be able to load or update the options value using an instance of this service.

Any problem if we go with it for an initial PR? We can keep looking and rename if we find something better.

[Eilon]

Ok I admit it's actually growing on me (but that might just be the 🍺 talking). But yes a PR would be good anyway. Have plenty of time to rename if needed.

[glennc]

This is a ping to make everyone think about the name again. All the ideas I have had are technically incorrect for one reason or other. So I am hoping someone throws out something amazing... :)

[matthewDDennis]

Looking at what it does, I almost feels like an OptionsCache in that it holds a value that can be invalidated based on a watcher or trigger. There isn't a timeout, but that would be easy to implement.

The problem is that the name OptionsCache is already used, but I think it is the thing with the wrong name. It is really an OptionsBuilder/factory or DeferredOptionsBuilder.

Just my 2 cents.

[divega]

I still positively like IOptionsSnapshot<TOptions>. I think @HaoK came up with it first. It is definitely just a snapshot of the options.

[HaoK]

@DamianEdwards how's the stew taste now?

[glennc]

Go with Snapshot. It is the least bad of all the bad options. @DamianEdwards agrees.

[HaoK]

acecacf

[rsnj]

I'm not sure if this is by design, but it's really confusing and just took me about an hour to debug. There are two different AddOptions extension methods in two different class libraries. In order to get this to work I had to reference Microsoft.Extensions.Options.ConfigurationExtensions in my project.json. Shouldn't there just be a single extension method for consistency?

Microsoft.Extensions.Options does not hook up the IOptionsMonitor
https://github.com/aspnet/Options/blob/rel/1.1.0-preview1/src/Microsoft.Extensions.Options/OptionsServiceCollectionExtensions.cs#L40

Microsoft.Extensions.Options.ConfigurationExtensions does:
https://github.com/aspnet/Options/blob/rel/1.1.0-preview1/src/Microsoft.Extensions.Options.ConfigurationExtensions/OptionsConfigurationServiceCollectionExtensions.cs#L22

Once I figured out the wrong method was being called, everything worked well.

[HaoK]

Yes this is how things are supposed to work, unless you bind options to an configuration instance, there's no way for the monitoring to be set up.

[divega]

Just to add to what @HaoK already said, keep in mind that configuration is right now the only sources of changes that Options knows about. If that changes in the future we may add sugar methods to register other IChangeTokenSource implementations.

Re the separate package for Microsoft.Extensions.OptionsConfigurationExtensions, that was done to reduce the coupling (see #97) between components that only need to depend on Options and packages in Configuration. We try to make it less confusing by adding the package in our default application templates whenever both Options and Configuration are included, but there might be cases in which we don't (and that may be a template bug).