Opened 14 months ago

Closed 14 months ago

Last modified 14 months ago

#34369 closed Cleanup/optimization (wontfix)

Improve the interaction between transaction.atomic() and generator functions

Reported by: Raphaël Barrois Owned by: nobody
Component: Database layer (models, ORM) Version: 4.1
Severity: Normal Keywords: atomic generator
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When wrapping a generator function with @transaction.atomic(), the decorator doesn't apply during the generator's execution.

This is a standard behaviour of all decorators for all generators — when the decorator calls the decorated function, that function returns immediately with the generator object — and doesn't execute even a single line of code.

Thus, in the following code example:

@transaction.atomic()
def frobnicate_users(qs):
  for user in qs.select_for_update():
    yield user.spam_it()

The code will fail at runtime, since the .select_for_update() is called after returning from the @transaction.atomic() block.

I believe it would make sense for Django to improve this interaction; possible options would be:

  • Having transaction.atomic() use inspect.isgeneratorfunction(wrapped) to raise when it is decorating a generator function (with guidance to replace it with a with transaction.atomic() in the function body);
  • Having transaction.atomic() automatically adjust itself to generator functions — if the wrapped function is a generator function, place the atomic block around a new generator wrapping the returned generator;
  • Have transaction.atomic() raise a warning if the value returned by its wrapped function is a generator.

It might make sense to add an optional kwarg to transaction.atomic() to control that behaviour — for instance, this improvement should not alter the handling of StreamingResponse.

If there is a consensus on moving this forward, I will happily provide a related pull-request.

Change History (3)

comment:1 by Mariusz Felisiak, 14 months ago

Type: UncategorizedCleanup/optimization

Thanks for the report, however I'm not sure it's worth additional complexity. This is clearly a misuse of the decorator, I'm not convinced that we need to document or tweak anything here.

comment:2 by Carlton Gibson, 14 months ago

Resolution: wontfix
Status: newclosed

I'm going to close based on Mariusz' comment. I tend to agree. The docs that pretty clear that the transaction is closed when the block exits, so this is expected behaviour. (Maybe it's just me but, in all these years, I never thought to use a generator in this context.)

comment:3 by Raphaël Barrois, 14 months ago

Fair enough; I guess this should instead be seen as an issue in contextlib, which could detect when it's decorating generators and adjust its behaviour accordingly.

Note: See TracTickets for help on using tickets.
Back to Top