From c74386cc3a9641e34e96cbdb7b28b7cad5a29815 Mon Sep 17 00:00:00 2001 From: Tim Schilling Date: Tue, 3 May 2022 07:20:14 -0500 Subject: [PATCH] Rename SQLPanel context var to control SQL access. The current build started failing for py3.8+ because the inner exception SQLQueryTriggered was raised in the test test_cursor_wrapper_asyncio_ctx. This was identified as being caused by sql.tracking.recording being set to false. The name recording made it seem as if this context var was controlling whether recording was occurring or not. However, it's true functionality was preventing SQL queries from being sent during the template panel's processing. Renaming that context var to allow_sql is more indicative of its purpose and makes the fixed test change clearer. --- debug_toolbar/panels/sql/tracking.py | 9 ++++++--- debug_toolbar/panels/templates/panel.py | 6 +++--- tests/panels/test_sql.py | 15 ++++++++------- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/debug_toolbar/panels/sql/tracking.py b/debug_toolbar/panels/sql/tracking.py index e3b225e9a..93304b21f 100644 --- a/debug_toolbar/panels/sql/tracking.py +++ b/debug_toolbar/panels/sql/tracking.py @@ -13,7 +13,10 @@ except ImportError: PostgresJson = None -recording = contextvars.ContextVar("debug-toolbar-recording", default=True) +# Prevents SQL queries from being sent to the DB. It's used +# by the TemplatePanel to prevent the toolbar from issuing +# additional queries. +allow_sql = contextvars.ContextVar("debug-toolbar-allow-sql", default=True) class SQLQueryTriggered(Exception): @@ -32,7 +35,7 @@ def cursor(*args, **kwargs): # See: # https://github.com/jazzband/django-debug-toolbar/pull/615 # https://github.com/jazzband/django-debug-toolbar/pull/896 - if recording.get(): + if allow_sql.get(): wrapper = NormalCursorWrapper else: wrapper = ExceptionCursorWrapper @@ -43,7 +46,7 @@ def chunked_cursor(*args, **kwargs): # solves https://github.com/jazzband/django-debug-toolbar/issues/1239 cursor = connection._djdt_chunked_cursor(*args, **kwargs) if not isinstance(cursor, BaseCursorWrapper): - if recording.get(): + if allow_sql.get(): wrapper = NormalCursorWrapper else: wrapper = ExceptionCursorWrapper diff --git a/debug_toolbar/panels/templates/panel.py b/debug_toolbar/panels/templates/panel.py index 0615a7601..1c2c96e09 100644 --- a/debug_toolbar/panels/templates/panel.py +++ b/debug_toolbar/panels/templates/panel.py @@ -13,7 +13,7 @@ from django.utils.translation import gettext_lazy as _ from debug_toolbar.panels import Panel -from debug_toolbar.panels.sql.tracking import SQLQueryTriggered, recording +from debug_toolbar.panels.sql.tracking import SQLQueryTriggered, allow_sql from debug_toolbar.panels.templates import views # Monkey-patch to enable the template_rendered signal. The receiver returns @@ -118,7 +118,7 @@ def _store_template_info(self, sender, **kwargs): value.model._meta.label, ) else: - token = recording.set(False) + token = allow_sql.set(False) try: saferepr(value) # this MAY trigger a db query except SQLQueryTriggered: @@ -130,7 +130,7 @@ def _store_template_info(self, sender, **kwargs): else: temp_layer[key] = value finally: - recording.reset(token) + allow_sql.reset(token) pformatted = pformat(temp_layer) self.pformat_layers.append((context_layer, pformatted)) context_list.append(pformatted) diff --git a/tests/panels/test_sql.py b/tests/panels/test_sql.py index 2445827c7..9824a1bec 100644 --- a/tests/panels/test_sql.py +++ b/tests/panels/test_sql.py @@ -18,7 +18,6 @@ from ..base import BaseTestCase from ..models import PostgresJSON -from ..sync import database_sync_to_async def sql_call(use_iterator=False): @@ -98,19 +97,21 @@ async def test_cursor_wrapper_async(self, mock_wrapper): wraps=sql_tracking.NormalCursorWrapper, ) async def test_cursor_wrapper_asyncio_ctx(self, mock_wrapper): - self.assertTrue(sql_tracking.recording.get()) + self.assertTrue(sql_tracking.allow_sql.get()) await sync_to_async(sql_call)() async def task(): - sql_tracking.recording.set(False) - # Calling this in another context requires the db connections - # to be closed properly. - await database_sync_to_async(sql_call)() + sql_tracking.allow_sql.set(False) + # By disabling sql_tracking.allow_sql, we are indicating that any + # future SQL queries should be stopped. If SQL query occurs, + # it raises an exception. + with self.assertRaises(sql_tracking.SQLQueryTriggered): + await sync_to_async(sql_call)() # Ensure this is called in another context await asyncio.create_task(task()) # Because it was called in another context, it should not have affected ours - self.assertTrue(sql_tracking.recording.get()) + self.assertTrue(sql_tracking.allow_sql.get()) self.assertEqual(mock_wrapper.call_count, 1) def test_generate_server_timing(self):