Opened 8 years ago

Closed 8 years ago

#22873 closed Cleanup/optimization (fixed)

Rename BaseDatabaseWrapper.use_debug_cursor to force_debug_cursor

Reported by: john@… Owned by: anonymous
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no


In Django 1.6, the logic for using the CursorDebugWrapper was:

  • If db.connection.use_debug_cursor is True, use it
  • If db.connection.use_debug_cursor is None, defer to settings.DEBUG
  • if db.connection.use_debug_cursor is False, don't use it.

This was changed in the fix for #20420. Commit [127218b92bb8] changed the logic to:

  • If db.connection.use_debug_cursor is True, use it
  • If db.connection.use_debug_cursor is False or None, defer to settings.DEBUG

I am using a management command to load lots of data into the database, and I was setting use_debug_cursor to False to disable query logging before my lengthy import. I'd rather not have to remember to set DEBUG=False every time I run a management command on my dev box. I'd like the 1.6 trinary behavior, as well as the better queries_logged property introduced in [40bfd8561d016]

Attachments (1)

trinary.patch (892 bytes) - added by john@… 8 years ago.
Proposed patch for django/db/backends/

Download all attachments as: .zip

Change History (9)

Changed 8 years ago by john@…

Attachment: trinary.patch added

Proposed patch for django/db/backends/

comment:1 Changed 8 years ago by Aymeric Augustin

Indeed, use_debug_cursor was a tri-valued boolean. That ranks quite high on my scale of programming anti-patterns. Furthermore, one of the three values was never used by Django.

So I changed it to a regular boolean. The new logic is:

  • If db.connection.use_debug_cursor is a true-ish value, use a debug cursor;
  • If db.connection.use_debug_cursor is a false-ish value, defer to settings.DEBUG.

None is just a false-ish value among others.

I should have changed the name to something like force_debug_cursor but I didn't find a short and explicit name so I left it as is. Suggestions?

I don't think the tri-valued boolean behaviour was introduced on purpose because it wasn't documented. It was a private API with no guarantee that its behaviour will be preserved across versions.

Accepting to fix this ticket would mean turning use_debug_cursor into a public API with its previous three-valued behaviour. I'm very close to -1 on this, but I'll let someone else confirm in case that behaviour was there for a good reason. If it was, it must be documented.

comment:2 Changed 8 years ago by john@…

This was added way back in 2010 in [5506653] by Alex Gaynor to support tests for assertNumQueries, fixing #5416. Previously, the code directly looked at settings.DEBUG. It may have been the intention to allow more control over the new debug cursor, but the most direct reason was to support the new assertion without forcing the test to run with DEBUG=True. assertNumQueries might now be broken when queries pass 9000, but that's probably rare and silly.

If use_debug_cursor it could initialized to settings.DEBUG in the __init__ method, then it could be set or cleared by code as needed. I'm not familiar with early settings loading, so it is possible that settings isn't initialized at this point.

If the current behavior is retained, force_debug_cursor is a better name, making it clearer that this is a private hook for testing.

If this undocumented API goes away, I could instead fail the management command when DEBUG=True.

comment:3 Changed 8 years ago by John Whitlock

It appears no one has anything to say about this bug. I'd support closing it w/o changes. For my own use case, I can detect and monkey patch queries_logged, and get the same desired behavior.

comment:4 Changed 8 years ago by Tim Graham

If the backwards compatibility concerns are minimal, renaming the attribute to force_debug_cursor seems like it could be a useful change to clarify the expected behavior.

comment:5 Changed 8 years ago by anonymous

Owner: changed from nobody to anonymous
Status: newassigned

comment:6 Changed 8 years ago by Areski Belaid

I'm also in favor to rename to force_debug_cursor for clarity.
Here a pull request for this:

comment:7 Changed 8 years ago by Tim Graham

Summary: Allow disabling debug cursor when DEBUG=TrueRename BaseDatabaseWrapper.use_debug_cursor to force_debug_cursor
Triage Stage: UnreviewedReady for checkin
Type: BugCleanup/optimization

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

Resolution: fixed
Status: assignedclosed

In 90faa196f67863370a4682ecfd053e189cd46705:

Fixed #22873 -- Renamed use_debug_cursor to force_debug_cursor to clarify the behavior.

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