[Django] #21029: Schema editors leak transaction state on exception
Django
----------------------------------------------+--------------------
Reporter: koniiiik | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
When a schema editor is used as a context manager and an uncaught
exception is raised within its block, `BaseDatabaseSchemaEditor`'s
`__exit__` method doesn't call `atomic.__exit__`, which leaves the
transaction opened when entering the block open.
I'm going to open a pull request shortly with a simple patch which does
fix things for me, but I'm not sure about its correctness and I have no
idea how to test this. An alternative approach might be to store an
instance of `Atomic` as an attribute in `BaseDatabaseSchemaEditor` and
call its `__enter__` and `__exit__` instead of creating separate
instances; I have no idea whether that makes any difference or not.
--
Ticket URL: <https://code.djangoproject.com/ticket/21029>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: koniiiik | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
I'm probably to blame for suggesting the `__enter__` / `__exit__` hack to
Andrew, but I have to say this is very bad.
If we're using this hack, we must guarantee that `__exit__` will be called
with robust exception handling.
--
Ticket URL: <https://code.djangoproject.com/ticket/21029#comment:1>
Django
Reporter: koniiiik | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by koniiiik):
The pull request can be found here:
https://github.com/django/django/pull/1545
--
Ticket URL: <https://code.djangoproject.com/ticket/21029#comment:2>
Django
Reporter: koniiiik | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timo):
* has_patch: 0 => 1
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/21029#comment:3>
Django
Reporter: koniiiik | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by tuxcanfly):
Added a simple test:
https://github.com/koniiiik/django/pull/2
As expected, the test fails on master, but passes with the patch (by
koniiiik) applied.
--
Ticket URL: <https://code.djangoproject.com/ticket/21029#comment:4>
Django
Reporter: koniiiik | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* component: Database layer (models, ORM) => Migrations
Comment:
Fix was committed in 630eb0564abd228da439d86ad93acb4089d795e7, but not the
test.
--
Ticket URL: <https://code.djangoproject.com/ticket/21029#comment:5>
Django
Reporter: koniiiik | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Andrew Godwin <andrew@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"6f7977bb63ea592faaa7b3bdf2898f8361f30260"]:
{{{
#!CommitTicketReference repository=""
revision="6f7977bb63ea592faaa7b3bdf2898f8361f30260"
Fixed #21029: Test for previously-commited SchemaEditor.__exit__ bug.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21029#comment:6>