Allow transaction.atomic to work in async contexts.

[alex]

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?

[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?

Wouldn't it be possible to add something like...

Did you try this, perhaps with some test cases to exercise the new code?

[alex]

ok sorry was in a hurry. The code isn't tested yet so I cannot assure it is working.
Details follow

[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 for starting and commiting the transaction.
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):

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.

[Carlton Gibson]

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.)