[Django] #24318: Can't configure Postgres isolation level if using recent psycopg2
Django
-------------------------------------+-------------------------------------
Reporter: carljm | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: 1.7
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
According to [https://docs.djangoproject.com/en/dev/ref/databases
/#isolation-level our docs], you should be able to set
`OPTIONS['isolation_level']` on a database connection to configure the
Postgres isolation level. But according to a report on django-users, which
I confirmed by code inspection, on at least Django 1.7+ setting
`OPTIONS['isolation_level']` has no effect if you are using psycopg2
2.4.2+.
The only place the configured `isolation_level` is referenced is in the
backend's
[https://github.com/django/django/blob/master/django/db/backends/postgresql_psycopg2/base.py#L198
_set_autocommit()] method, and there it is only used if using an older
psycopg2.
In 1.7 we had a `_set_isolation_level()` method on the PG backend, but it
was never called, and has since been removed in master.
This bug probably dates back to the transaction changes in Django 1.6,
though I haven't confirmed that.
--
Ticket URL: <https://code.djangoproject.com/ticket/24318>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: carljm | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: aaugustin (added)
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
It looks like we could use
[http://initd.org/psycopg/docs/connection.html#connection.set_session
connection.set_session()] to also set the isolation level in the `if
self.psycopg2_version >= (2, 4, 2)` branch.
Aymeric, was this simply an oversight?
--
Ticket URL: <https://code.djangoproject.com/ticket/24318#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: carljm | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by aaugustin):
Historically support for isolation levels in the psycopg2 backend only
existed for people who wanted to switch to autocommit. This is now the
default.
That said, I'm not sure that any isolation level other than the default
actually works well with Django, and I don't remember if the docs reflect
that.
--
Ticket URL: <https://code.djangoproject.com/ticket/24318#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: carljm | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by carljm):
I think `get_or_create` may have issues on a different isolation level?
Otherwise I'm not aware of areas where Django would work poorly, but there
certainly may be others. AFAIK the default level for MySQL is "REPEATABLE
READ" (whereas for PostgreSQL it's "READ COMMITTED"), and I can't see
anywhere we change that in the MySQL backend. And for SQLite AFAIK the
only available isolation level is SERIALIZABLE. So unless I'm missing
something I think Django core already implicitly "supports" three
different transaction isolation levels, depending on your database
backend.
I'm not sure the feature historically existed _only_ for switching to
autocommit - I've run into people who wanted "REPEATABLE READ" on
PostgreSQL for whatever reason (often because they were converting a
project from MySQL).
I don't feel strongly, but we should either remove the documentation about
setting the isolation level, or make it possible again, and given that
it's a small change I'd slightly favor the latter. Perhaps the docs should
be updated to note that Django does not test against PostgreSQL with any
transaction isolation level other than the default.
--
Ticket URL: <https://code.djangoproject.com/ticket/24318#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: carljm | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by carljm):
Of course, if this bug really has existed since 1.6 and wasn't noticed
until now, that's a strong indication that nobody is actually using this
feature (or if they are, they can't tell the difference when it doesn't
work.)
--
Ticket URL: <https://code.djangoproject.com/ticket/24318#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: carljm | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timgraham):
I guess [e0449316] preceded the rest of the transactions overhaul in 1.6.
I'd favor removing the "Isolation level" section from the docs rather than
saying "this only works with psycopg2 < 2.4.2" as that seems a bit odd to
explain. What do you think?
--
Ticket URL: <https://code.djangoproject.com/ticket/24318#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: carljm | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by aaugustin):
psycopg2 2.4.2+ correctly separates the notions of autocommit and
isolation levels. I don't want to mix them again in Django
We can fix this bug by setting the isolation level with
[http://initd.org/psycopg/docs/connection.html#connection.set_session
set_session] in init_connection_state.
We could also tell people to configure the default isolation level in
their database server's configuration, but since it's easy to fix the
regression, I don't like this solution.
--
Ticket URL: <https://code.djangoproject.com/ticket/24318#comment:6>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: 1.7
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => aaugustin
* cc: aaugustin (removed)
* status: new => assigned
Comment:
For the record here's the report on django-users:
https://groups.google.com/d/msg/django-users/HTc5HNwa4iQ/hMCaGsWhc2YJ
--
Ticket URL: <https://code.djangoproject.com/ticket/24318#comment:7>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: 1.7
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/4132
The patch is quite straightforward. I'm confident it's the correct
solution. Can someone double-check?
I will backport the first commit of the pull request to 1.7 and 1.8.
--
Ticket URL: <https://code.djangoproject.com/ticket/24318#comment:8>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: 1.7
(models, ORM) |
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24318#comment:9>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: 1.7
(models, ORM) |
Severity: Release blocker | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Ready for checkin => Accepted
Comment:
Sorry, my first attempt wasn't very good. So much for confidence. I
updated the PR with a second attempt.
--
Ticket URL: <https://code.djangoproject.com/ticket/24318#comment:10>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 1.7
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"76356d963c131252e1ce4285083fd21fd0bdedd9"]:
{{{
#!CommitTicketReference repository=""
revision="76356d963c131252e1ce4285083fd21fd0bdedd9"
Fixed #24318 -- Set the transaction isolation level with psycopg >= 2.4.2.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24318#comment:11>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 1.7
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"87e9cad4a4481e7cd91e2e7bac46efe54000a932"]:
{{{
#!CommitTicketReference repository=""
revision="87e9cad4a4481e7cd91e2e7bac46efe54000a932"
[1.8.x] Fixed #24318 -- Set the transaction isolation level with psycopg
>= 2.4.2.
Backport of 76356d96 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24318#comment:12>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 1.7
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"9b7d512d5fb88844d920929b3cbb9d6c38b8b891"]:
{{{
#!CommitTicketReference repository=""
revision="9b7d512d5fb88844d920929b3cbb9d6c38b8b891"
[1.7.x] Fixed #24318 -- Set the transaction isolation level with psycopg
>= 2.4.2.
Backport of 76356d96 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24318#comment:13>