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

None of the OnXxx hooks are async-friendly #1069

Closed
tanveerbadar opened this issue Jan 13, 2020 · 2 comments · Fixed by #1172
Closed

None of the OnXxx hooks are async-friendly #1069

tanveerbadar opened this issue Jan 13, 2020 · 2 comments · Fixed by #1172

Comments

@tanveerbadar
Copy link

tanveerbadar commented Jan 13, 2020

None of the OnXxx hooks on IRegistrationBuilder are async friendly. Here's the problem I am facing right now.

OnActivated(args =>
            {
                var context = args.Instance;
                var cts = new CancellationTokenSource();
                var token = cts.Token;
                context.Entities.RemoveRange(context.Entities);
                context.SaveAsync(token).ConfigureAwait(false).GetAwaiter().GetResult();

                context.Entities.Add(new BasicEntity { Id = 1, Name = "abc" });
                context.Entities.Add(new BasicEntity { Id = 2, Name = "def" });
                context.Entities.Add(new BasicEntity { Id = 3, Name = "ghi" });
                context.SaveAsync(token).ConfigureAwait(false).GetAwaiter().GetResult();
            }

If the idea sounds good - to add Func<T, Task> based overloads - you can assign this issue to me for implementation.

@alistairjevans
Copy link
Member

This does somewhat line up with the async Disposal support we've just added. There is a certain symmetry to supporting async OnActivated as well.

I'm not sure if we'd want any other methods than OnActivated to be made async-friendly, because I don't think OnActivating, OnPreparing or OnReleasing should be implied to be something that can take a while to run.

One thing to bear in mind is that we probably don't want to implement a completely asynchronous chain throughout the Resolve path (that would be a pretty huge set of changes), and would suggest that Resolve generally can take a while to run.

@tillig
Copy link
Member

tillig commented Jan 14, 2020

I don't have any personal thoughts on this; I think it'd be a fine feature. I do think OnRelease might also need async treatment (to take advantage of async disposal in a more manual capacity, maybe). I don't know that I'd hold 5.0 up for it.

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

Successfully merging a pull request may close this issue.

3 participants