#31286: Database specific fields checks should be databases aware.
-------------------------------------+-------------------------------------
Reporter: Hongtao Ma | Owner: Hongtao
| Ma
Type: Bug | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Thanks for the analysis Tim
> This check queries the database server, but as far as I can tell, it
doesn't require the presence of the test database.
The mode is effectively not a database specific setting but it is
nonetheless a value that must be retrieved using a query (`SELECT
@@sql_mode` to be precise) which means that a connection must exists
against the server and if no test database is setup this leaves the
testing machinery no choice but to connect directly to the non-test
database which is frowned upon in a testing scenario. Now that we don't
systematically create / setup for reuse test databases when the suite
subset doesn't need it (#28478) we need a way to ensure we don't issue
database queries against un-prepared databases.
> These checks do static analysis (checking field attributes), and while
some of these checks may rely on a connection to the database server to
populate connection.features, for example, I don't believe these checks
require the presence of the test database.
Similar scenario in this case, they may or may not require to issue
database queries to perform these checks and we likely don't want them to
be performed against non-test databases.
> They are really static checks that don't require the presence of a
database.
That's the crux of the issue I believe. They may or may not as they are
opaque to the caller and its this opacity that forces setups where only a
subset of the databases to be safely available (e.g. test runs) to limit
their scope.
As I've shared in comment:9 I'm in agreement with you here though, I do
believe that we should default to having unspecified `database` subset
mean ''all databases'' instead of ''no databases'' and not only for
`makemigrations`. What I mean is that we should be doing something like
the following
{{{!diff
diff --git a/django/core/checks/database.py
b/django/core/checks/database.py
index d125f1eaa6..91265ac966 100644
--- a/django/core/checks/database.py
+++ b/django/core/checks/database.py
@@ -4,9 +4,7 @@
@register(Tags.database)
-def check_database_backends(databases=None, **kwargs):
- if databases is None:
- return []
+def check_database_backends(databases, **kwargs):
issues = []
for alias in databases:
conn = connections[alias]
diff --git a/django/core/checks/registry.py
b/django/core/checks/registry.py
index 3139fc3ef4..a285abbc3f 100644
--- a/django/core/checks/registry.py
+++ b/django/core/checks/registry.py
@@ -1,6 +1,7 @@
from collections.abc import Iterable
from itertools import chain
+from django.db import connections
from django.utils.inspect import func_accepts_kwargs
@@ -85,6 +86,9 @@ def run_checks(
if tags is not None:
checks = [check for check in checks if not
set(check.tags).isdisjoint(tags)]
+ if databases is None:
+ databases = list(connections)
+
for check in checks:
new_errors = check(app_configs=app_configs,
databases=databases)
if not isinstance(new_errors, Iterable):
diff --git a/django/core/management/commands/check.py
b/django/core/management/commands/check.py
index e61cff79f3..531e059c37 100644
--- a/django/core/management/commands/check.py
+++ b/django/core/management/commands/check.py
@@ -46,7 +46,7 @@ def add_arguments(self, parser):
action="append",
choices=tuple(connections),
dest="databases",
- help="Run database related checks against these aliases.",
+ help="Run database related checks only against these
aliases.",
)
def handle(self, *app_labels, **options):
}}}
I've given the full suite [https://github.com/django/django/pull/19912 a
go here]. Note that this preserves the behaviour of commands that directly
operate against a database and default to `[DB_DEFAULT_ALIAS]` such as
`migrate` as it's likely that not all databases are available in the
context they are run.
--
Ticket URL: <https://code.djangoproject.com/ticket/31286#comment:11>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/010701999779a4e2-1370af6e-1dca-4214-88aa-7d22f0f40fe1-000000%40eu-central-1.amazonses.com.
