#17671 closed Bug (fixed)
CursorWrapper in Python 2.7 cannot be used as a contextmanager
Reported by: | Michael Manfre | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.3 |
Severity: | Normal | Keywords: | regression |
Cc: | denisenkom | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The following code pattern for executing raw SQL works fine in Python 2.6.x, but fails in Python 2.7.x.
from django.db import connection with connection.cursor() as c: c.execute('select 1')
Python 2.7.x results in
Traceback (most recent call last): File "C:\projects\web-site\test\withcursor\myapp\tests.py", line 21, in test_with_cursor with connection.cursor() as c: AttributeError: __exit__
I was able to reproduce this with a new project using sqlite3 and a new app without any models.
Attachments (1)
Change History (15)
comment:1 by , 13 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 13 years ago
Has patch: | set |
---|---|
Resolution: | invalid |
Status: | closed → reopened |
To fix this add following methods to django.db.backends.CursorWrapper
def enter(self):
return self
def exit(self, type, value, traceback):
return self.cursor.exit(type, value, traceback)
comment:3 by , 13 years ago
Cc: | added |
---|
comment:4 by , 13 years ago
Here is more detailed description of underlying problem with python 2.7
http://stackoverflow.com/questions/4146095/getattr-doesnt-work-with-enter-attributeerror
comment:5 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | reopened → new |
comment:6 by , 12 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
This code should never have worked; see http://bugs.python.org/issue9220 (and its duplicate http://bugs.python.org/issue9259).
I'm having a hard time determining if we should change something in Django:
- our documentation states that the cursors implement the standard Python DB-API described in PEP 249
- but PEP 249 doesn't say whether cursors are supposed to work as context managers
- since Python 2.6 sqlite's cursors act as context managers for transaction control,
- but Django implements its own transaction control
__enter__
and __exit__
methods that just forward to __enter__
and __exit__
wouldn't hurt, but I can't find a real reason to implement them.
comment:7 by , 12 years ago
The underlying Python "regression" and whether or not the Django code should have ever worked is not really relevant. The important question is, was it the intention that cursors in Django should be usable as a contextmanager? If so, then this is clearly a regression. If not, then is there a reason to explicitly block the functionality provided by the underlying cursors?
The expectation for django-mssql has always been that cursors, which must be used more frequently than with the core backends, should be usable as a contextmanager for the sake of code brevity and readability. I have applied a (hopefully temporary monkey) patch to django-mssql v1.1 to maintain this expected behavior.
comment:8 by , 12 years ago
Hmm, IMHO there is reason to block the forwarding. The reason is that what the forwarding does is backend specific, and thus could act as transaction control mechanism for example, but depending on the backend.
A shortcut of using "with connection.cursor() as c: ..." in place of "c = connection.cursor(); try: ... finally: c.close()" could be nice.
comment:9 by , 12 years ago
Having a context manager with consistent behavior for the backends makes sense. Adding a patch with documentation to point out the changing behaviors.
by , 12 years ago
Attachment: | django-ticket17671.diff added |
---|
context manager shortcut without pass-thru
comment:10 by , 12 years ago
Owner: | changed from | to
---|
comment:11 by , 12 years ago
Triage Stage: | Design decision needed → Accepted |
---|
I am marking this as accepted, the semantics should be that:
with connection.cursor() as c: do_something
is equivalent to
c = connection.cursor() try: do_something finally: c.close()
That is, we avoid database specific implementations like SQLite's transaction management semantics.
comment:12 by , 11 years ago
Didn't realize the previous patch didn't make it in to 1.5. Here's a pull request against master.
comment:13 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
AFAIK Django only wraps the cursor obtained from the underlying database library. It might be that your 2.7 installation does not use the same library as the 2.6 installation, hence the error.
I was personally able to reproduce the same error with my Python 2.6 installation, which enforces my opinion about an underlying library issue: