[Django] #30005: Example in documentation of transaction.atomic uses both decorator and context manager

3 views
Skip to first unread message

Django

unread,
Dec 3, 2018, 9:05:03 PM12/3/18
to django-...@googlegroups.com
#30005: Example in documentation of transaction.atomic uses both decorator and
context manager
-----------------------------------------+------------------------
Reporter: Peter Hull | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 2.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-----------------------------------------+------------------------
In the documentation of transaction.atomic
(https://docs.djangoproject.com/en/2.1/topics/db/transactions
/#controlling-transactions-explicitly), the example below the line
"Wrapping atomic in a try/except block allows for natural handling of
integrity errors" uses both the decorator and the context manager versions
of atomic(). This wouldn't necessarily be a problem except that using the
decorator here causes the example to do exactly what the warning a couple
paragraphs later says not to do: "Avoid catching exceptions inside
atomic!" I suggest that removing the use of the decorator in the example
would be the most prudent edit?

--
Ticket URL: <https://code.djangoproject.com/ticket/30005>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Dec 3, 2018, 9:29:25 PM12/3/18
to django-...@googlegroups.com
#30005: Example in documentation of transaction.atomic uses both decorator and
context manager
-------------------------------+------------------------------------

Reporter: Peter Hull | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 2.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by Tim Graham):

* stage: Unreviewed => Accepted


Comment:

I guess it's a typo in the original commit:
d7bc4fbc94df6c231d71dffa45cf337ff13512ee.

--
Ticket URL: <https://code.djangoproject.com/ticket/30005#comment:1>

Django

unread,
Dec 3, 2018, 10:05:45 PM12/3/18
to django-...@googlegroups.com
#30005: Example in documentation of transaction.atomic uses both decorator and
context manager
-------------------------------+--------------------------------------
Reporter: Peter Hull | Owner: Peter Hull
Type: Bug | Status: assigned
Component: Documentation | Version: 2.1

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by Peter Hull):

* owner: nobody => Peter Hull
* status: new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/30005#comment:2>

Django

unread,
Dec 3, 2018, 10:11:04 PM12/3/18
to django-...@googlegroups.com
#30005: Example in documentation of transaction.atomic uses both decorator and
context manager
-------------------------------+--------------------------------------
Reporter: Peter Hull | Owner: Peter Hull
Type: Bug | Status: assigned
Component: Documentation | Version: 2.1

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by Peter Hull):

* has_patch: 0 => 1


Comment:

[https://github.com/django/django/pull/10717 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/30005#comment:3>

Django

unread,
Dec 4, 2018, 5:04:52 AM12/4/18
to django-...@googlegroups.com
#30005: Example in documentation of transaction.atomic uses both decorator and
context manager
-------------------------------+--------------------------------------
Reporter: Peter Hull | Owner: Peter Hull
Type: Bug | Status: assigned
Component: Documentation | Version: 2.1

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+--------------------------------------

Comment (by Simon Charette):

I don't think the example is wrong at all as the inner
`transaction.atomic` block creates a `SAVEPOINT` (because
`atomic(savepoint)` defaults to `True`) and thus the connection is usable
in the event of an `IntegrityError` because a rollback of the savepoint
would happen.

What the example is trying to demonstrate is that you can still nest
`transaction.atomic()` block safely if you wrap code expected to raise
database error appropriately and have queries within it's context be
atomic. In this case the `create_parent()` and `add_children()` calls will
be executed in the same transaction because of the function decorator.

Think of it this way, `atomic` would be completely impossible to use if
you couldn't nest calls this way given each calls to the database can
theoretically raise a database exception.

--
Ticket URL: <https://code.djangoproject.com/ticket/30005#comment:4>

Django

unread,
Dec 6, 2018, 3:10:32 PM12/6/18
to django-...@googlegroups.com
#30005: Example in documentation of transaction.atomic uses both decorator and
context manager
-------------------------------+--------------------------------------
Reporter: Peter Hull | Owner: Peter Hull
Type: Bug | Status: closed
Component: Documentation | Version: 2.1
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by Tim Graham):

* status: assigned => closed
* resolution: => invalid


--
Ticket URL: <https://code.djangoproject.com/ticket/30005#comment:5>

Reply all
Reply to author
Forward
0 new messages