[Django] #32172: Adapt signals to allow async handlers
Django
------------------------------------------+------------------------
Reporter: Carlton Gibson | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Keywords: async
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------+------------------------
Currently (circa 3.1) we have a couple of `sync_to_async()` usages in
[https://github.com/django/django/blob/009fddc96b88a9aee1232d6a2637f2970fdcfb50/django/core/handlers/asgi.py#L154
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,
[https://github.com/django/django/blob/76181308fb02e67794d0cc1471766a5d7e4c877e/django/core/handlers/base.py#L26
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, perhaps on a PR, or the
[https://forum.djangoproject.com/c/internals/async/8 Forum's Async
category] but opening this ticket to track efforts.
--
Ticket URL: <https://code.djangoproject.com/ticket/32172>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Carlton Gibson | Owner:
| Konstantin Volkov
Type: New feature | Status: assigned
Component: Core (Other) | Version: master
Keywords: async | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Konstantin Volkov):
* owner: nobody => Konstantin Volkov
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32172#comment:1>
Django
Reporter: Carlton Gibson | Owner:
| Konstantin Volkov
Type: New feature | Status: assigned
Keywords: async | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/32172#comment:2>
Django
Reporter: Carlton Gibson | Owner:
| Konstantin Volkov
Type: New feature | Status: assigned
Keywords: async | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Carlton Gibson:
Old description:
> Currently (circa 3.1) we have a couple of `sync_to_async()` usages in
> [https://github.com/django/django/blob/009fddc96b88a9aee1232d6a2637f2970fdcfb50/django/core/handlers/asgi.py#L154
> 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,
> [https://github.com/django/django/blob/76181308fb02e67794d0cc1471766a5d7e4c877e/django/core/handlers/base.py#L26
> 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, perhaps on a PR, or the
> [https://forum.djangoproject.com/c/internals/async/8 Forum's Async
> category] but opening this ticket to track efforts.
New description:
Currently (circa 3.1) we have a couple of `sync_to_async()` usages in
[https://github.com/django/django/blob/009fddc96b88a9aee1232d6a2637f2970fdcfb50/django/core/handlers/asgi.py#L154
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,
[https://github.com/django/django/blob/76181308fb02e67794d0cc1471766a5d7e4c877e/django/core/handlers/base.py#L26
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 [https://forum.djangoproject.com/c/internals/async/8 Forum's
Async category] but opening this ticket to track efforts.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32172#comment:3>
Django
Reporter: Carlton Gibson | Owner:
| Konstantin Volkov
Type: New feature | Status: assigned
Keywords: async | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/13651 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/32172#comment:4>
Django
Reporter: Carlton Gibson | Owner:
| Konstantin Volkov
Type: New feature | Status: assigned
Keywords: async | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Accepted => Ready for checkin
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32172#comment:5>
Django
Reporter: Carlton Gibson | Owner:
| Konstantin Volkov
Type: New feature | Status: assigned
Keywords: async | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
Comment:
Performance issues are
[https://github.com/django/django/pull/13651#issuecomment-731185122 not
looking good at first glance] — so we need a proper story there to move
forward. Will mark as PNI whilst we discuss.
--
Ticket URL: <https://code.djangoproject.com/ticket/32172#comment:6>
Django
Reporter: Carlton Gibson | Owner:
| Konstantin Volkov
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 1 => 0
* has_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/32172#comment:7>
Django
Reporter: Carlton Gibson | Owner:
| Konstantin Volkov
Type: New feature | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Janzen):
* cc: Jon Janzen (added)
Comment:
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
--
Ticket URL: <https://code.djangoproject.com/ticket/32172#comment:8>
Django
Reporter: Carlton Gibson | Owner:
| Konstantin Volkov
Type: New feature | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jon Janzen):
Hey Konstantin,
I'm interested in working on this ticket, would you mind if I claimed it?
--
Ticket URL: <https://code.djangoproject.com/ticket/32172#comment:9>
Django
Reporter: Carlton Gibson | Owner: Jon Janzen
Type: New feature | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Janzen):
* owner: Konstantin Volkov => Jon Janzen
Comment:
[https://github.com/django/django/pull/16547 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/32172#comment:10>
Django
Reporter: Carlton Gibson | Owner: Jon Janzen
Type: New feature | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
The [https://github.com/django/django/pull/16547 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32172#comment:11>
Django
Reporter: Carlton Gibson | Owner: Jon Janzen
Type: New feature | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: async | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32172#comment:12>
Django
Reporter: Carlton Gibson | Owner: Jon
| Janzen
Type: New feature | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32172#comment:13>
Django
Reporter: Carlton Gibson | Owner: Jon
| Janzen
Component: Core (Other) | Version: dev
Keywords: async | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e83a88566a71a2353cebc35992c110be0f8628af" e83a8856]:
{{{
#!CommitTicketReference repository=""
revision="e83a88566a71a2353cebc35992c110be0f8628af"
Fixed #32172 -- Adapted signals to allow async handlers.
co-authored-by: kozzztik <kozz...@mail.ru>
co-authored-by: Carlton Gibson <carlton...@noumenal.es>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32172#comment:14>
Django
Reporter: Carlton Gibson | Owner: Jon
| Janzen
Type: New feature | Status: closed
Component: Core (Other) | Version: dev
Severity: Normal | Resolution: fixed
Keywords: async | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by GitHub <noreply@…>):
In [changeset:"9b224579875e30203d079cc2fee83b116d98eb78" 9b224579]:
{{{
#!CommitTicketReference repository=""
revision="9b224579875e30203d079cc2fee83b116d98eb78"
Refs #32172 -- Used asgiref coroutine shim in async signals tests.
Bug in e83a88566a71a2353cebc35992c110be0f8628af.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32172#comment:15>