Opened 3 years ago
Closed 3 years ago
#33038 closed Cleanup/optimization (wontfix)
Documentation for exception handling and transactions is misleading.
Reported by: | Rich | Owned by: | nobody |
---|---|---|---|
Component: | Documentation | Version: | 3.2 |
Severity: | Normal | Keywords: | transactions exceptions |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
This section of the doc:
https://docs.djangoproject.com/en/3.2/topics/db/transactions/#controlling-transactions-explicitly
Has a warning "Avoid catching exceptions inside atomic!"
I believe this warning and the associated text is an oversimplification and leads to confusion. For example, this is a very common pattern:
with transaction.atomic(): try: thing = query.get() except ObjectDoesNotExist: # Create a new instance or whatever
The documentation implies that this is to be avoided, despite it being a clean and performant alternative to using query.exists(). The Django source code itself uses this pattern.
To resolve this I'd like to see it made clear that there is a difference between exceptions which break the transaction (e.g. DatabaseError), and those which don't - the point is already made in the doc about DatabaseError but no counter example of the acceptable uses of exception handling within a transaction is given.
Change History (1)
comment:1 by , 3 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Type: | Uncategorized → Cleanup/optimization |
Thanks for the report, however I don't see anything confusing in this note.
This warning describes the class of exceptions that should not be caught. IMO a counterexample is not necessary and could make this (already long) note more confusing.
As far as I'm aware this is not a common pattern, I also couldn't find it in Django itself. It's not sth that we would like to recommend in Django docs.