Skip to content
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

Make IAsyncConfigureOptions<TOptions>behave similar to IConfigureOptions<TOptions> #15165

Open
Piedone opened this issue Jan 25, 2024 · 5 comments
Milestone

Comments

@Piedone
Copy link
Member

Piedone commented Jan 25, 2024

Is your feature request related to a problem? Please describe.

While the name suggests it, IAsyncConfigureOptions<TOptions> is not the async equivalent of IConfigureOptions<TOptions>, since it works slightly differently: it allows you to inject TOptions, not IOptions<TOptions>.

Related: #15125.

Describe the solution you'd like

Make it more similar by adding an IAsyncOptions<T> type to inject, with GetValueAsync(). This would otherwise work the same as IOptions<T> and its Value property.

Describe alternatives you've considered

Rename the service, to like IAsyncConfigurationProvider and in its interface docs emphasize the similarity but differences to IConfigureOptions<TOptions>.

@MikeAlhayek
Copy link
Member

@Piedone I do not disagree with the request. I always thought the .net framework should have async support with IOptions<> pattern because some sites want to retrieve settings from the external sources in order to configure their options. In OC we configure configure many of our services from the database. But I think we should first understand why did Microsoft not implement IConfigureOptionAsync<> as part of the IOptions<> design. There has to be a reason why they did not. Maybe @sebastienros has more insights on why not.

In our case, almost all the calls to _siteService.GetSiteSettingsAsync().GetAwaiter().GetResult() are not blocking even when they use GetAwaiter().GetResult(). Only the very first call to GetSiteSettingsAsync() will block once during startup. But all the other calls will not block since they return data from the memory.

@Piedone
Copy link
Member Author

Piedone commented Jan 26, 2024

I guess not much news on this from since 2018, when it was "we are not going to implement this at this time".

This might be more impactful if you (as we discussed under #15089) run an app with many tenants that are periodically shut down, resulting in frequent shell starts. This is the case of a SaaS.

@MikeAlhayek
Copy link
Member

yes which is why I do not disagree with you. But, before you go down that rabbit hole, it may worth understanding why they said "
we are not going to implement this at this time." Maybe we need to submit an issue aspnetcore to see what would they say to our argument of needing this.

@MikeAlhayek
Copy link
Member

@Piedone I submitted an issue dotnet/runtime#97575 to see what information would we get back. Please chime in there if you have more reasons why they should support async configuration in the framework.

@Piedone
Copy link
Member Author

Piedone commented Jan 26, 2024

Great, thanks, watching it. I have nothing to add, since you explained it, but I'll chime in if necessary.

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

No branches or pull requests

3 participants