From fbd16438f46bc2128926958ad24331da5d1b406f Mon Sep 17 00:00:00 2001 From: Florian Zimmermann Date: Thu, 1 Jun 2023 16:39:52 +0200 Subject: [PATCH] Fixed #33143 -- Raised RuntimeWarning when performing import-time queries. --- django/db/backends/utils.py | 14 ++++ docs/ref/applications.txt | 31 +++++++ docs/releases/5.0.txt | 3 + tests/apps/query_performing_app/__init__.py | 0 tests/apps/query_performing_app/apps.py | 92 +++++++++++++++++++++ tests/apps/tests.py | 85 ++++++++++++++++++- tests/inspectdb/models.py | 5 +- 7 files changed, 227 insertions(+), 3 deletions(-) create mode 100644 tests/apps/query_performing_app/__init__.py create mode 100644 tests/apps/query_performing_app/apps.py diff --git a/django/db/backends/utils.py b/django/db/backends/utils.py index 4c1e62bcbf..0aa94cc30b 100644 --- a/django/db/backends/utils.py +++ b/django/db/backends/utils.py @@ -3,9 +3,11 @@ import decimal import functools import logging import time +import warnings from contextlib import contextmanager from hashlib import md5 +from django.apps import apps from django.db import NotSupportedError from django.utils.dateparse import parse_time @@ -19,6 +21,12 @@ class CursorWrapper: WRAP_ERROR_ATTRS = frozenset(["fetchone", "fetchmany", "fetchall", "nextset"]) + APPS_NOT_READY_WARNING_MSG = ( + "Accessing the database during app initialization is discouraged. To fix this " + "warning, avoid executing queries in AppConfig.ready() or when your app " + "modules are imported." + ) + def __getattr__(self, attr): cursor_attr = getattr(self.cursor, attr) if attr in CursorWrapper.WRAP_ERROR_ATTRS: @@ -53,6 +61,8 @@ class CursorWrapper: "Keyword parameters for callproc are not supported on this " "database backend." ) + if not apps.ready: + warnings.warn(self.APPS_NOT_READY_WARNING_MSG, category=RuntimeWarning) self.db.validate_no_broken_transaction() with self.db.wrap_database_errors: if params is None and kparams is None: @@ -80,6 +90,8 @@ class CursorWrapper: return executor(sql, params, many, context) def _execute(self, sql, params, *ignored_wrapper_args): + if not apps.ready: + warnings.warn(self.APPS_NOT_READY_WARNING_MSG, category=RuntimeWarning) self.db.validate_no_broken_transaction() with self.db.wrap_database_errors: if params is None: @@ -89,6 +101,8 @@ class CursorWrapper: return self.cursor.execute(sql, params) def _executemany(self, sql, param_list, *ignored_wrapper_args): + if not apps.ready: + warnings.warn(self.APPS_NOT_READY_WARNING_MSG, category=RuntimeWarning) self.db.validate_no_broken_transaction() with self.db.wrap_database_errors: return self.cursor.executemany(sql, param_list) diff --git a/docs/ref/applications.txt b/docs/ref/applications.txt index fc4caba669..03063f2086 100644 --- a/docs/ref/applications.txt +++ b/docs/ref/applications.txt @@ -431,6 +431,11 @@ application registry. It must be called explicitly in other cases, for instance in plain Python scripts. + .. versionchanged:: 5.0 + + Raises a ``RuntimeWarning`` when apps interact with the database before + the app registry has been fully populated. + .. currentmodule:: django.apps The application registry is initialized in three stages. At each stage, Django @@ -509,3 +514,29 @@ Here are some common problems that you may encounter during initialization: :setting:`INSTALLED_APPS` to contain ``'django.contrib.admin.apps.SimpleAdminConfig'`` instead of ``'django.contrib.admin'``. + +* ``RuntimeWarning: Accessing the database during app initialization is + discouraged.`` This warning is triggered for database queries executed before + apps are ready, such as during module imports or in the + :meth:`AppConfig.ready` method. Such premature database queries are + discouraged because they will run during the startup of every management + command, which will slow down your project startup, potentially cache stale + data, and can even fail if migrations are pending. + + For example, a common mistake is making a database query to populate form + field choices:: + + class LocationForm(forms.Form): + country = forms.ChoiceField(choices=[c.name for c in Country.objects.all()]) + + In the example above, the query from ``Country.objects.all()`` is executed + during module import, because the ``QuerySet`` is iterated over. To avoid the + warning, the form could use a :class:`~django.forms.ModelChoiceField` + instead:: + + class LocationForm(forms.Form): + country = forms.ModelChoiceField(queryset=Country.objects.all()) + + To make it easier to find the code that triggered this warning, you can make + Python :ref:`treat warnings as errors ` to reveal the + stack trace, for example with ``python -Werror manage.py shell``. diff --git a/docs/releases/5.0.txt b/docs/releases/5.0.txt index dc31aa703a..0b65ee189a 100644 --- a/docs/releases/5.0.txt +++ b/docs/releases/5.0.txt @@ -577,6 +577,9 @@ Miscellaneous * Support for ``cx_Oracle`` < 8.3 is removed. +* Executing SQL queries before the app registry has been fully populated now + raises :exc:`RuntimeWarning`. + .. _deprecated-features-5.0: Features deprecated in 5.0 diff --git a/tests/apps/query_performing_app/__init__.py b/tests/apps/query_performing_app/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/apps/query_performing_app/apps.py b/tests/apps/query_performing_app/apps.py new file mode 100644 index 0000000000..f57ccbb7a0 --- /dev/null +++ b/tests/apps/query_performing_app/apps.py @@ -0,0 +1,92 @@ +from django.apps import AppConfig +from django.db import connections + + +class BaseAppConfig(AppConfig): + name = "apps.query_performing_app" + database = "default" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.query_results = [] + + def ready(self): + self.query_results = [] + self._perform_query() + + def _perform_query(self): + raise NotImplementedError + + +class ModelQueryAppConfig(BaseAppConfig): + def _perform_query(self): + from ..models import TotallyNormal + + queryset = TotallyNormal.objects.using(self.database) + queryset.update_or_create(name="new name") + self.query_results = list(queryset.values_list("name")) + + +class QueryDefaultDatabaseModelAppConfig(ModelQueryAppConfig): + database = "default" + + +class QueryOtherDatabaseModelAppConfig(ModelQueryAppConfig): + database = "other" + + +class CursorQueryAppConfig(BaseAppConfig): + def _perform_query(self): + connection = connections[self.database] + with connection.cursor() as cursor: + cursor.execute("SELECT 42" + connection.features.bare_select_suffix) + self.query_results = cursor.fetchall() + + +class QueryDefaultDatabaseCursorAppConfig(CursorQueryAppConfig): + database = "default" + + +class QueryOtherDatabaseCursorAppConfig(CursorQueryAppConfig): + database = "other" + + +class CursorQueryManyAppConfig(BaseAppConfig): + def _perform_query(self): + from ..models import TotallyNormal + + connection = connections[self.database] + table_meta = TotallyNormal._meta + with connection.cursor() as cursor: + cursor.executemany( + "INSERT INTO %s (%s) VALUES(%%s)" + % ( + connection.introspection.identifier_converter(table_meta.db_table), + connection.ops.quote_name(table_meta.get_field("name").column), + ), + [("test name 1",), ("test name 2",)], + ) + self.query_results = [] + + +class QueryDefaultDatabaseCursorManyAppConfig(CursorQueryManyAppConfig): + database = "default" + + +class QueryOtherDatabaseCursorManyAppConfig(CursorQueryManyAppConfig): + database = "other" + + +class StoredProcedureQueryAppConfig(BaseAppConfig): + def _perform_query(self): + with connections[self.database].cursor() as cursor: + cursor.callproc("test_procedure") + self.query_results = [] + + +class QueryDefaultDatabaseStoredProcedureAppConfig(StoredProcedureQueryAppConfig): + database = "default" + + +class QueryOtherDatabaseStoredProcedureAppConfig(StoredProcedureQueryAppConfig): + database = "other" diff --git a/tests/apps/tests.py b/tests/apps/tests.py index ecfb70162f..e443e37dc5 100644 --- a/tests/apps/tests.py +++ b/tests/apps/tests.py @@ -1,11 +1,18 @@ import os +from unittest.mock import patch +import django from django.apps import AppConfig, apps from django.apps.registry import Apps from django.contrib.admin.models import LogEntry from django.core.exceptions import AppRegistryNotReady, ImproperlyConfigured -from django.db import models -from django.test import SimpleTestCase, override_settings +from django.db import connections, models +from django.test import ( + SimpleTestCase, + TransactionTestCase, + override_settings, + skipUnlessDBFeature, +) from django.test.utils import extend_sys_path, isolate_apps from .models import SoAlternative, TotallyNormal, new_apps @@ -539,3 +546,77 @@ class NamespacePackageAppTests(SimpleTestCase): with self.settings(INSTALLED_APPS=["nsapp.apps.NSAppConfig"]): app_config = apps.get_app_config("nsapp") self.assertEqual(app_config.path, self.app_path) + + +class QueryPerformingAppTests(TransactionTestCase): + available_apps = ["apps"] + databases = {"default", "other"} + expected_msg = ( + "Accessing the database during app initialization is discouraged. To fix this " + "warning, avoid executing queries in AppConfig.ready() or when your app " + "modules are imported." + ) + + def test_query_default_database_using_model(self): + query_results = self.run_setup("QueryDefaultDatabaseModelAppConfig") + self.assertSequenceEqual(query_results, [("new name",)]) + + def test_query_other_database_using_model(self): + query_results = self.run_setup("QueryOtherDatabaseModelAppConfig") + self.assertSequenceEqual(query_results, [("new name",)]) + + def test_query_default_database_using_cursor(self): + query_results = self.run_setup("QueryDefaultDatabaseCursorAppConfig") + self.assertSequenceEqual(query_results, [(42,)]) + + def test_query_other_database_using_cursor(self): + query_results = self.run_setup("QueryOtherDatabaseCursorAppConfig") + self.assertSequenceEqual(query_results, [(42,)]) + + def test_query_many_default_database_using_cursor(self): + self.run_setup("QueryDefaultDatabaseCursorManyAppConfig") + + def test_query_many_other_database_using_cursor(self): + self.run_setup("QueryOtherDatabaseCursorManyAppConfig") + + @skipUnlessDBFeature("create_test_procedure_without_params_sql") + def test_query_default_database_using_stored_procedure(self): + connection = connections["default"] + with connection.cursor() as cursor: + cursor.execute(connection.features.create_test_procedure_without_params_sql) + + try: + self.run_setup("QueryDefaultDatabaseStoredProcedureAppConfig") + finally: + with connection.schema_editor() as editor: + editor.remove_procedure("test_procedure") + + @skipUnlessDBFeature("create_test_procedure_without_params_sql") + def test_query_other_database_using_stored_procedure(self): + connection = connections["other"] + with connection.cursor() as cursor: + cursor.execute(connection.features.create_test_procedure_without_params_sql) + + try: + self.run_setup("QueryOtherDatabaseStoredProcedureAppConfig") + finally: + with connection.schema_editor() as editor: + editor.remove_procedure("test_procedure") + + def run_setup(self, app_config_name): + custom_settings = override_settings( + INSTALLED_APPS=[f"apps.query_performing_app.apps.{app_config_name}"] + ) + # Ignore the RuntimeWarning, as override_settings.enable() calls + # AppConfig.ready() which will trigger the warning. + with self.assertWarnsMessage(RuntimeWarning, self.expected_msg): + custom_settings.enable() + try: + with patch.multiple(apps, ready=False, loading=False, app_configs={}): + with self.assertWarnsMessage(RuntimeWarning, self.expected_msg): + django.setup() + + app_config = apps.get_app_config("query_performing_app") + return app_config.query_results + finally: + custom_settings.disable() diff --git a/tests/inspectdb/models.py b/tests/inspectdb/models.py index 25714cb086..ad42871644 100644 --- a/tests/inspectdb/models.py +++ b/tests/inspectdb/models.py @@ -1,5 +1,6 @@ from django.db import connection, models from django.db.models.functions import Lower +from django.utils.functional import SimpleLazyObject class People(models.Model): @@ -94,7 +95,9 @@ class JSONFieldColumnType(models.Model): } -test_collation = connection.features.test_collations.get("non_default") +test_collation = SimpleLazyObject( + lambda: connection.features.test_collations.get("non_default") +) class CharFieldDbCollation(models.Model):