[Django] #33882: async transaction.atomic
Django
-----------------------------------------+------------------------
Reporter: alex | Owner: nobody
Type: New feature | Status: new
Component: Uncategorized | Version: 4.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
Wouldn't it be possible to add something like:
__aenter__ = sync_to_async(Atomic.__enter__, thread_sensitive=True)
__aexit__ = sync_to_async(Atomic.__exit__, thread_sensitive=True)
to the Atomic class to support async calls?
--
Ticket URL: <https://code.djangoproject.com/ticket/33882>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------+--------------------------------------
Reporter: alex | Owner: nobody
Type: New feature | Status: new
Component: Uncategorized | Version: 4.0
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Hi Alex.
Can I ask you to take the time to explain your issue report in full
please? (Likely it's clear to you, but it needs more detail, perhaps
starting from the beginning…)
Is it a duplicate of #32409?
--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:1>
Django
-------------------------------+--------------------------------------
Reporter: alex | Owner: nobody
Component: Uncategorized | Version: 4.0
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
* status: new => closed
* resolution: => needsinfo
--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:2>
Django
-------------------------------+--------------------------------------
Reporter: alex | Owner: nobody
Component: Uncategorized | Version: 4.0
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
* status: closed => new
* resolution: needsinfo =>
Comment:
ok sorry was in a hurry. The code isn't tested yet so I cannot assure it
is working.
Details follow
--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:3>
Django
-------------------------------+--------------------------------------
Reporter: alex | Owner: nobody
Type: New feature | Status: new
Component: Uncategorized | Version: 4.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Comment (by alex):
django supports db transactions with rollback via the
django.db.transaction.atomic function which can either be used as
decorator or as contextmanager.
Everything wrapped within is executed as transaction, see:
https://docs.djangoproject.com/en/4.0/topics/db/transactions/
That is very nice but now there is a ''little'' problem:
it isn't async safe:
https://forum.djangoproject.com/t/is-it-possible-to-use-transaction-
atomic-with-async-functions/8924
When looking through the contextmanager (which is actually a class named
Atomic in the same file) I saw that __enter__ and __exit__ use simple db
operations.
If we use the wrappers like suggested (`__aenter__` and `__aexit__`) then
everything should be fine in theory. In praxis I need to test the code (I
have not much time so it may be easier if somebody else tries this out)
I build a wrapper class like this (of course it would be better to have it
in Atomic itself so the atomic function can be used and no manual
initialization is required):
{{{
#!python
from asgiref.sync import sync_to_async
from django.db.transaction import Atomic
class AsyncAtomic(Atomic):
__aenter__ = sync_to_async(Atomic.__enter__, thread_sensitive=True)
__aexit__ = sync_to_async(Atomic.__exit__, thread_sensitive=True)
}}}
'''Warning''': untested
I must confess: the use case is very rare and is only useful in
combination with select_for_update for delaying row updates (otherwise it
is an antipattern as it causes slow transactions). And this is why I
haven't a good example code.
--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: alex | Owner: nobody
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
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 Carlton Gibson):
* keywords: => async
* component: Uncategorized => Database layer (models, ORM)
* stage: Unreviewed => Accepted
Comment:
OK, thanks for the update Alex. I'm going to Accept this, since it's a
desirable feature that transactions work in an async-context.
I'm half-inclined towards closing as `needsinfo` or marking as
`Someday/Maybe` as I suspect the implementation will require a bit more
than just adding the `sync_to_async()` wrappers. 🤔
I guess the first step would be to write some test cases for that and see
what issues arise and what the performance looks like. (From there it's
easier to see what's really involved.)
--
Ticket URL: <https://code.djangoproject.com/ticket/33882#comment:5>