#32172 closed New feature (fixed)
Adapt signals to allow async handlers
| Reported by: | Carlton Gibson | Owned by: | Jon Janzen |
|---|---|---|---|
| Component: | Core (Other) | Version: | dev |
| Severity: | Normal | Keywords: | async |
| Cc: | Andrew Godwin, Jon Janzen | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Currently (circa 3.1) we have a couple of sync_to_async() usages in the main line of `ASGIHandler.__call__()`, for the request life cycle signals:
await sync_to_async(signals.request_started.send, thread_sensitive=True)(sender=self.__class__, scope=scope)
Much as is done of the middleware, adapting to accept either synchronous or asynchronous middleware classes, it would be good to be able to push that sync_to_async() call down into the send() method, allowing signal handlers to be either sync (as now) or async coroutines.
This would give us the option of having a completely async request pathway, if we were to remove all synchronous middleware and signal handlers.
There will need to be some discussion of exact implementation, perhaps on a PR, or the Forum's Async category but opening this ticket to track efforts.
Change History (15)
comment:1 by , 4 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:2 by , 4 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:3 by , 4 years ago
| Description: | modified (diff) |
|---|
comment:4 by , 4 years ago
| Has patch: | set |
|---|
comment:5 by , 4 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
I'm going to mark this RFC — I don't think it's far away.
We want to do a small amount of profiling before we say the final OK, but it can live off the review list for a while during that.
comment:6 by , 4 years ago
| Patch needs improvement: | set |
|---|---|
| Triage Stage: | Ready for checkin → Accepted |
Performance issues are not looking good at first glance — so we need a proper story there to move forward. Will mark as PNI whilst we discuss.
comment:7 by , 2 years ago
| Has patch: | unset |
|---|---|
| Patch needs improvement: | unset |
comment:8 by , 21 months ago
| Cc: | added |
|---|
I hope I'm not spamming this ticket, but I posted on the Forum about asyncifying the auth app which would involve this ticket so I thought it would be a good idea to CC the ticket on this issue: https://forum.djangoproject.com/t/asyncifying-django-contrib-auth-and-signals-and-maybe-sessions/18770
comment:9 by , 21 months ago
Hey Konstantin,
I'm interested in working on this ticket, would you mind if I claimed it?
comment:11 by , 21 months ago
The new PR looks promising: any async receivers are dispatch concurrently.
I'll (briefly) see if others have capacity to review, otherwise I'd be inclined to push it forwards.
comment:12 by , 21 months ago
| Has patch: | set |
|---|
comment:13 by , 21 months ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
PR