There is a definite disparency about what the code does and what the docs say: """ If the block of code is successfully completed, the changes are committed to the database. If there is an exception, the changes are rolled back. """
If an exception is catched inside the block, then there should be no rollback. With PostgreSQL you can't continue to use the transaction, but in other databases you can. For these other databases allowing use of the transaction, yet still rolling back it later on seems to be a footgun - why are you allowed to continue to use the transaction if a rollback is going to happen anyways? - Anssi On Thursday, September 19, 2013 4:33:53 PM UTC+3, Aymeric Augustin wrote: > > 2013/9/19 Richard Ward <ric...@richard.ward.name <javascript:>> > >> So what is the problem here? I assume it is one of: >> > >> 1. 'innocent_looking_function' is badly written: it should not be >> catching IntegrityErrors under any circumstances (though that seems like >> a >> valid thing to do), it should instead use something like get_or_create. >> >> > It's possible to catch IntegrityErrors. The correct pattern — regardless > of function calls — is: > > try: > with transaction.atomic(): > do_stuff() > except IntegrityError: > # transaction.atomic has already performed the rollback at this point > handle_error() > > It's documented: > https://docs.djangoproject.com/en/dev/topics/db/transactions/#django.db.transaction.atomic > >> >> 1. 'innocent_looking_function' should have 'with >> transaction.atomic():' just inside the 'try' block, and presumably so >> should every other bit of code where an IntegrityError is caught. But >> what >> if 'innocent_looking_function' also gets used elsewhere that may not be >> inside an atomic block? >> >> Indeed that would implement the pattern shown above and resolve the > problem. > >> >> 1. transaction.atomic is in some way buggy / oddly designed. >> >> As the designer of this API, I say: suggestions welcome :) Read below > before jumping to the drawing board, though. > >> IMO this behaviour makes code that looks valid (to me) not work in the >> expected way. >> > Catching IntegrityError inside transaction.atomic is wrong because it > hides the exception from transaction.atomic. > > As a consquence, transaction.atomic believe that everything went well and > merrily commits. But the database has a broken transaction in progress; it > can't commit.. transaction.atomic fails to commit, rolls back, and raises > an exception — which isn't the original IntegrityError, adding to confusion. > >> In case its relevant I'm using InnoDB tables on MySQL. >> > You might have received more meaningful errors with another database > engine, but overall you've described the intended, cross-database behavior > of transaction.atomic. > >> So Is there a bug? Or am I using this incorrectly, or misunderstanding >> something? >> > The code works as designed. It relies on exceptions to distinguish failure > from success. This is the cleanest way to implement transactions as a > context manager. If you swallow the exception, you make a failure look like > a success, and things go wrong. > > The docs talk a lot about exceptions but they don't warn > specifically about this pitfall. It would be a worthy addition. Can you > file a ticket? > > Thank you, > > -- > Aymeric. > -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at http://groups.google.com/group/django-developers. For more options, visit https://groups.google.com/groups/opt_out.
