[Django] #20897: Asymmetrical cursor creation in djnago.db.backeds.BaseDatabaseWrapper
Django
----------------------------------------------+--------------------
Reporter: emil.filipov@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Database layer (models, ORM) | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
We currently have the following cursor creation code in
BaseDatabaseWrapper:
{{{
def cursor(self):
self.validate_thread_sharing()
if (self.use_debug_cursor or
(self.use_debug_cursor is None and settings.DEBUG)):
cursor = self.make_debug_cursor(self._cursor())
else:
cursor = util.CursorWrapper(self._cursor(), self)
return cursor
}}}
The issue I have with this piece of code is that the cursor is created in
a very different way, depending on whether we have a debug mode or not.
The symmetrical way of doing it would be to call something like
''self.make_regular_cursor(self._cursor())'' for the cursor creating in
the regular case.
Use case:
Modules like Django Debug Toolbar wrap the ''.cursor()'' invocation on the
base class, it's all working nicely for the standard backends, that do not
override ''cursor()''. The problem, however, becomes apparent when you
write your own backend, that overrides the ''cursor()'' method. Then you
have to call the parent method, to ensure compatibility with DDT, but that
parent method will either return the generic Cursor class (for a regular
cursor), or your own, custom Cursor class (for a debug cursor, where you
have overridden ''make_debug_cursor()'') - a situation requiring a
decision based on the type of the returned argument.
Patch:
Fairly trivial - just add another member function and wrap
''util.CursorWrapper(self._cursor(), self)'' inside it. I can provide one
if needed.
--
Ticket URL: <https://code.djangoproject.com/ticket/20897>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: emil.filipov@… | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.5
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by akaariai):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* needs_tests: => 0
* needs_docs: => 0
Comment:
Why not... Please make a pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/20897#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: emil.filipov@… | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.5
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: timmartin (added)
* has_patch: 0 => 1
Comment:
I've created a pull request at https://github.com/django/django/pull/2649
I opted to call `self._cursor()` in both branches and pass the result into
`make_regular_cursor`. This isn't exactly what was suggested above, but I
think it's more regular. I can't quite picture the use case described, so
if there's something about this that won't satisfy the use case let me
know.
--
Ticket URL: <https://code.djangoproject.com/ticket/20897#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: emil.filipov@… | Owner: nobody
Type: | Status: closed
Cleanup/optimization | Version: 1.5
Component: Database layer | Resolution: fixed
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"27aa85246a218b0999c7c9d227eed2afb08ed510"]:
{{{
#!CommitTicketReference repository=""
revision="27aa85246a218b0999c7c9d227eed2afb08ed510"
Fixed #20897 -- Added make_cursor() for consistent cursor creation
In django.db.backends.BaseDatabaseWrapper, pulled the creation of
cursors in the non-debug case into a separate method, in order to
make behavior more consistent when overriding the cursor creation
in derived classes.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20897#comment:3>