Opened 5 years ago

Closed 3 years ago

Last modified 3 years ago

#17671 closed Bug (fixed)

CursorWrapper in Python 2.7 cannot be used as a contextmanager

Reported by: 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


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\", 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)

django-ticket17671.diff (3.7 KB) - added by manfre 4 years ago.
context manager shortcut without pass-thru

Download all attachments as: .zip

Change History (15)

comment:1 Changed 5 years ago by claudep

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to 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:

In [1]: from django.db import connection

In [2]: with connection.cursor() as c:
   ...:     c.execute('select 1')
AttributeError                            Traceback (most recent call last)

/home/claude/checkouts/django-git/mysite/<ipython console> in <module>()

/home/claude/virtualenvs/djangogit/lib/python2.6/site-packages/django/db/backends/util.pyc in __getattr__(self, attr)
     26             return self.__dict__[attr]
     27         else:
---> 28             return getattr(self.cursor, attr)
     30     def __iter__(self):

AttributeError: 'SQLiteCursorWrapper' object has no attribute '__exit__'

comment:2 Changed 4 years ago by denisenkom@…

  • Has patch set
  • Resolution invalid deleted
  • Status changed from closed to 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)

Version 0, edited 4 years ago by denisenkom@… (next)

comment:3 Changed 4 years ago by denisenkom

  • Cc denisenkom added

comment:4 Changed 4 years ago by denisenkom

Here is more detailed description of underlying problem with python 2.7

comment:5 Changed 4 years ago by aaugustin

  • Owner changed from nobody to aaugustin
  • Status changed from reopened to new

comment:6 Changed 4 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Design decision needed

This code should never have worked; see (and its duplicate

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 Changed 4 years ago by manfre

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 Changed 4 years ago by akaariai

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 Changed 4 years ago by manfre

Having a context manager with consistent behavior for the backends makes sense. Adding a patch with documentation to point out the changing behaviors.

Changed 4 years ago by manfre

context manager shortcut without pass-thru

comment:10 Changed 4 years ago by aaugustin

  • Owner changed from aaugustin to nobody

comment:11 Changed 4 years ago by akaariai

  • Triage Stage changed from Design decision needed to Accepted

I am marking this as accepted, the semantics should be that:

with connection.cursor() as c:

is equivalent to

c = connection.cursor()

That is, we avoid database specific implementations like SQLite's transaction management semantics.

comment:12 Changed 3 years ago by manfre

Didn't realize the previous patch didn't make it in to 1.5. Here's a pull request against master.

comment:13 Changed 3 years ago by Anssi Kääriäinen <akaariai@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 99c87f1410106ce543a1a0332428afc472beef7f:

Fixed #17671 - Cursors are now context managers.

comment:14 Changed 3 years ago by Tim Graham <timograham@…>

In 0d02c5429970804068520f0dc6f0ae9fb3e08b9c:

Fixed #21207 -- Fixed test failure on Oracle: test_cursor_contextmanager

refs #17671

Note: See TracTickets for help on using tickets.
Back to Top