Browse Source

Fixed #33143 -- Raised RuntimeWarning when performing import-time queries.

Florian Zimmermann 1 year ago
parent
commit
fbd16438f4

+ 14 - 0
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)

+ 31 - 0
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 <python:warning-filter>` to reveal the
+  stack trace, for example with ``python -Werror manage.py shell``.

+ 3 - 0
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

+ 0 - 0
tests/apps/query_performing_app/__init__.py


+ 92 - 0
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"

+ 83 - 2
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()

+ 4 - 1
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):