-
Notifications
You must be signed in to change notification settings - Fork 9.9k
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 IAsyncStartup and IAsyncStartupFilter to support async startup classes and filters #5897
Comments
/cc @muratg |
Please do! I could use this now |
Don't change it, just add a new interface. |
That's basically what I meant, updating to clarify. |
Why does it have to be an error to have both? Couldn't you just have async be prioritized over sync? I.e. resolve |
I think async should win over sync |
We certainly could, it just seems like it's unnecessarily ambiguous. Wouldn't you have to explicitly register both to get in that scenario? All our helpers should be updated to register just the one ( Either way, I'm fine, as long as the behavior is clear. |
@anurse you may implement both on the same object. |
I suppose. I just still think that's a pretty unclear situation, since you'll have code that we'll never call ourselves. Anyway, prioritizing async is the right approach if we're going to allow both. |
@davidfowl The call stack for this doesn't make sense. Startup is instantiated and run from WebHostBuilder.Build(), not WebHost.StartAsync(), so Build() would have to become async for this. |
Yeah. Making Build async is out of scope for 2.0 I think, it's way too late. What about reopening #1085 and moving |
@muratg discussed this with @davidfowl offline, this should move to Backlog to revisit in 3.0. |
please try to be consistent with async naming conventions StartAsync() StopAsync() on Hosting IWebHost would suggest that interface naming should be IStartUpAsync and IStartupFilterAsync ? thoughts |
I don't think that makes it any more consistent than |
Any updates on this one? It's stuck for more than a year now.. |
@effyteva see https://github.com/aspnet/Hosting/issues/1088#issuecomment-311741002, this issue will be revisited for the 3.0 release. |
Will this actually be revisited for 3.0? |
Unfortunately no, we had other higher priority work that means this is not scheduled for 3.0. |
I spent a could of minutes looking at this and I don't think it;s possible to implement without blocking. We've basically deprecated IStartup in 3.0 so adding |
this is going to be a problem for blazor if it continues to use the same Startup mechanism. all i/o has to be async in blazor, and you can't just start a task to do it either - so right now there's no way to, say, initialise a service which depends on runtime-loaded data. |
Is there a reason Similar to what @gulbanana notes, there are a lot of scenarios where that would be handy. For example, I inherited several systems that use More generally, I understand why this change hasn't been made yet, but on the other hand, as people start getting the "async whenever possible" message, I'm seeing more and more I'm of the opinion that re-factoring |
Any progress on this? For 5.0 maybe? I have several use cases for loading config async (i.e. AWS Secrets Manager / Azure Key Vault) and |
There's no progress for 5.0 no, we don't know how to do this without blocking or breaking changes. It might be possible to run filters in 2 stages that never overlap. |
Hmm, that might actually work now that I've typed it out... |
Ah nope, the problem is the underlying call to Configure itself would need to be async OR sync. They can't interleave or overlap because of the calling pattern. One needs to replace the other. |
Say yes to breaking changes! Async all the way! |
Closing in favor of #24142 |
We should introduce a new set of interfaces:
IAsyncStartup
andIAsyncStartupFilter
with the following API surface:Startup classes will now support a
Task ConfigureAsync(IApplicationBuilder app)
method, which is mutually exclusive withvoid Configure(IApplicationBuilder app)
. It shall be an error to have both methods on a startup class.IAsyncStartup
andIAsyncStartupFilter
will be the primitives used by the entire Hosting pipeline. Adapters will exist in Hosting to allow the continued use ofIStartup
andIStartupFilter
in the public API surface (to reduce breaking changes).The text was updated successfully, but these errors were encountered: