#36714: Async signals lose ContextVar state due to use of asyncio.gather
----------------------------------+------------------------------------
Reporter: Mykhailo Havelia | Owner: (none)
Type: Uncategorized | Status: new
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: asyncio, signals | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
Comment (by Mykhailo Havelia):
Replying to [comment:1 Carlton Gibson]:
> The whole idea of that API is to allow control over this, when trying to
keep things structured, no?
🙂 If we’re talking about `ASGIHandler.handler`, then yes. It’s easy to
implement and would unblock my current work on the async backend.
> Then we should resolve #36315 first no?
I think our priority should be to focus first on the:
- database cursor
- ORM
- middlewares,
- signals
- cache
to ensure proper async handling first. After that, we can focus on
optimization and improvements since optimization often adds complexity,
and there are still some core challenges to solve in the async port 😔.
That said, if I can prepare a small MR with minimal changes that could be
reviewed and merged quickly, I’d really prefer that approach, especially
since `#36315` has been pending for over six months. It would help unblock
my current work.
If that’s not possible, I'm more than happy to contribute to `#36315` to
help move it forward and unblock these changes together.
What do you think?
> The whole idea of that API is to allow control over this, when trying to
keep things structured, no?
If we're talking about using `asyncio.gather` inside signals, things get a
bit more complicated. I'm not entirely sure how context sharing behaves
with parallel tasks yet (I'll need to look into it). We might need to do
something similar to what `asgiref` does, for example:
{{{
def _restore_context(context: contextvars.Context) -> None:
cvalue = context.get(cvar)
try:
if cvar.get() != cvalue:
cvar.set(cvalue)
except LookupError:
cvar.set(cvalue)
}}}
Manually handling context like this can easily lead to unexpected
behavior. For instance, overwriting a global variable when we shouldn't.
So we'll need to test this carefully, especially with `sync_to_async` and
`async_to_sync`. It might take a fair bit of time to review and verify
that everything works as expected.
It's much easier to delete it. What do you think?
--
Ticket URL: <https://code.djangoproject.com/ticket/36714#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/0107019a59b362fd-a0e57217-98c9-465a-91ed-e11b91defc89-000000%40eu-central-1.amazonses.com.
