Refs #31055 -- Added --database option to the check management command.

This avoids enabling the ``database`` checks unless they are explicitly
requested and allows to disable on a per-alias basis which is required
when only creating a subset of the test databases.

This also removes unnecessary BaseCommand._run_checks() hook.
This commit is contained in:
Simon Charette 2020-02-07 08:46:13 +01:00 committed by Mariusz Felisiak
parent 9cc743d0c8
commit 0b83c8cc4d
10 changed files with 66 additions and 56 deletions

View File

@ -4,8 +4,11 @@ from . import Tags, register
@register(Tags.database)
def check_database_backends(*args, **kwargs):
def check_database_backends(databases=None, **kwargs):
if databases is None:
return []
issues = []
for conn in connections.all():
for alias in databases:
conn = connections[alias]
issues.extend(conn.validation.check(**kwargs))
return issues

View File

@ -54,7 +54,7 @@ class CheckRegistry:
tags += (check,)
return inner
def run_checks(self, app_configs=None, tags=None, include_deployment_checks=False):
def run_checks(self, app_configs=None, tags=None, include_deployment_checks=False, databases=None):
"""
Run all registered checks and return list of Errors and Warnings.
"""
@ -63,13 +63,9 @@ class CheckRegistry:
if tags is not None:
checks = [check for check in checks if not set(check.tags).isdisjoint(tags)]
else:
# By default, 'database'-tagged checks are not run as they do more
# than mere static code analysis.
checks = [check for check in checks if Tags.database not in check.tags]
for check in checks:
new_errors = check(app_configs=app_configs)
new_errors = check(app_configs=app_configs, databases=databases)
assert is_iterable(new_errors), (
"The function %r did not return a list. All functions registered "
"with the checks registry must return a list." % check)

View File

@ -378,21 +378,20 @@ class BaseCommand:
self.stdout.write(output)
return output
def _run_checks(self, **kwargs):
return checks.run_checks(**kwargs)
def check(self, app_configs=None, tags=None, display_num_errors=False,
include_deployment_checks=False, fail_level=checks.ERROR):
include_deployment_checks=False, fail_level=checks.ERROR,
databases=None):
"""
Use the system check framework to validate entire Django project.
Raise CommandError for any serious message (error or critical errors).
If there are only light messages (like warnings), print them to stderr
and don't raise an exception.
"""
all_issues = self._run_checks(
all_issues = checks.run_checks(
app_configs=app_configs,
tags=tags,
include_deployment_checks=include_deployment_checks,
databases=databases,
)
header, body, footer = "", "", ""

View File

@ -32,6 +32,10 @@ class Command(BaseCommand):
'non-zero status. Default is ERROR.'
),
)
parser.add_argument(
'--database', action='append', dest='databases',
help='Run database related checks against these aliases.',
)
def handle(self, *app_labels, **options):
include_deployment_checks = options['deploy']
@ -62,4 +66,5 @@ class Command(BaseCommand):
display_num_errors=True,
include_deployment_checks=include_deployment_checks,
fail_level=getattr(checks, options['fail_level']),
databases=options['databases'],
)

View File

@ -2,7 +2,6 @@ import time
from importlib import import_module
from django.apps import apps
from django.core.checks import Tags, run_checks
from django.core.management.base import (
BaseCommand, CommandError, no_translations,
)
@ -20,8 +19,13 @@ from django.utils.text import Truncator
class Command(BaseCommand):
help = "Updates database schema. Manages both apps with migrations and those without."
requires_system_checks = False
def add_arguments(self, parser):
parser.add_argument(
'--skip-checks', action='store_true',
help='Skip system checks.',
)
parser.add_argument(
'app_label', nargs='?',
help='App label of an application to synchronize the state.',
@ -59,13 +63,11 @@ class Command(BaseCommand):
help='Creates tables for apps without migrations.',
)
def _run_checks(self, **kwargs):
issues = run_checks(tags=[Tags.database])
issues.extend(super()._run_checks(**kwargs))
return issues
@no_translations
def handle(self, *args, **options):
database = options['database']
if not options['skip_checks']:
self.check(databases=[database])
self.verbosity = options['verbosity']
self.interactive = options['interactive']
@ -77,8 +79,7 @@ class Command(BaseCommand):
import_module('.management', app_config.name)
# Get the database we're operating from
db = options['database']
connection = connections[db]
connection = connections[database]
# Hook for backends needing any database preparation
connection.prepare_database()

View File

@ -79,7 +79,8 @@ Django's system checks are organized using the following tags:
* ``database``: Checks database-related configuration issues. Database checks
are not run by default because they do more than static code analysis as
regular checks do. They are only run by the :djadmin:`migrate` command or if
you specify the ``database`` tag when calling the :djadmin:`check` command.
you specify configured database aliases using the ``--database`` option when
calling the :djadmin:`check` command.
* ``models``: Checks of model, field, and manager definitions.
* ``security``: Checks security related configuration.
* ``signals``: Checks on signal declarations and handler registrations.
@ -90,6 +91,11 @@ Django's system checks are organized using the following tags:
Some checks may be registered with multiple tags.
.. versionchanged:: 3.1
The ``database`` checks are now run only for database aliases specified
using the :option:`check --database` option.
Core system checks
==================

View File

@ -106,6 +106,16 @@ For example, to perform only models and compatibility checks, run::
django-admin check --tag models --tag compatibility
.. django-admin-option:: --database DATABASE
.. versionadded:: 3.1
Specifies the database to run checks requiring database access::
django-admin check --database default --database other
By default, these checks will not be run.
.. django-admin-option:: --list-tags
Lists all available tags.

View File

@ -229,7 +229,10 @@ Logging
Management Commands
~~~~~~~~~~~~~~~~~~~
* ...
* The new :option:`check --database` option allows specifying database aliases
for running the ``database`` system checks. Previously these checks were
enabled for all configured :setting:`DATABASES` by passing the ``database``
tag to the command.
Migrations
~~~~~~~~~~
@ -474,6 +477,12 @@ Miscellaneous
*What's New* :ref:`Security section <whats-new-security-3.1>` above for more
details.
* :djadmin:`check` management command now runs the ``database`` system checks
only for database aliases specified using :option:`check --database` option.
* :djadmin:`migrate` management command now runs the ``database`` system checks
only for a database to migrate.
.. _deprecated-features-3.1:
Features deprecated in 3.1

View File

@ -1,8 +1,7 @@
import unittest
from unittest import mock
from django.core.checks import Tags, run_checks
from django.core.checks.registry import CheckRegistry
from django.core.checks.database import check_database_backends
from django.db import connection
from django.test import TestCase
@ -10,30 +9,12 @@ from django.test import TestCase
class DatabaseCheckTests(TestCase):
databases = {'default', 'other'}
@property
def func(self):
from django.core.checks.database import check_database_backends
return check_database_backends
def test_database_checks_not_run_by_default(self):
"""
`database` checks are only run when their tag is specified.
"""
def f1(**kwargs):
return [5]
registry = CheckRegistry()
registry.register(Tags.database)(f1)
errors = registry.run_checks()
self.assertEqual(errors, [])
errors2 = registry.run_checks(tags=[Tags.database])
self.assertEqual(errors2, [5])
def test_database_checks_called(self):
with mock.patch('django.db.backends.base.validation.BaseDatabaseValidation.check') as mocked_check:
run_checks(tags=[Tags.database])
self.assertTrue(mocked_check.called)
@mock.patch('django.db.backends.base.validation.BaseDatabaseValidation.check')
def test_database_checks_called(self, mocked_check):
check_database_backends()
self.assertFalse(mocked_check.called)
check_database_backends(databases=self.databases)
self.assertTrue(mocked_check.called)
@unittest.skipUnless(connection.vendor == 'mysql', 'Test only for MySQL')
def test_mysql_strict_mode(self):
@ -47,7 +28,7 @@ class DatabaseCheckTests(TestCase):
'django.db.backends.utils.CursorWrapper.fetchone', create=True,
return_value=(response,)
):
self.assertEqual(self.func(None), [])
self.assertEqual(check_database_backends(databases=self.databases), [])
bad_sql_modes = ['', 'WHATEVER']
for response in bad_sql_modes:
@ -56,6 +37,6 @@ class DatabaseCheckTests(TestCase):
return_value=(response,)
):
# One warning for each database alias
result = self.func(None)
result = check_database_backends(databases=self.databases)
self.assertEqual(len(result), 2)
self.assertEqual([r.id for r in result], ['mysql.W002', 'mysql.W002'])

View File

@ -160,22 +160,22 @@ class CheckCommandTests(SimpleTestCase):
@override_system_checks([simple_system_check, tagged_system_check])
def test_simple_call(self):
call_command('check')
self.assertEqual(simple_system_check.kwargs, {'app_configs': None})
self.assertEqual(tagged_system_check.kwargs, {'app_configs': None})
self.assertEqual(simple_system_check.kwargs, {'app_configs': None, 'databases': None})
self.assertEqual(tagged_system_check.kwargs, {'app_configs': None, 'databases': None})
@override_system_checks([simple_system_check, tagged_system_check])
def test_given_app(self):
call_command('check', 'auth', 'admin')
auth_config = apps.get_app_config('auth')
admin_config = apps.get_app_config('admin')
self.assertEqual(simple_system_check.kwargs, {'app_configs': [auth_config, admin_config]})
self.assertEqual(tagged_system_check.kwargs, {'app_configs': [auth_config, admin_config]})
self.assertEqual(simple_system_check.kwargs, {'app_configs': [auth_config, admin_config], 'databases': None})
self.assertEqual(tagged_system_check.kwargs, {'app_configs': [auth_config, admin_config], 'databases': None})
@override_system_checks([simple_system_check, tagged_system_check])
def test_given_tag(self):
call_command('check', tags=['simpletag'])
self.assertIsNone(simple_system_check.kwargs)
self.assertEqual(tagged_system_check.kwargs, {'app_configs': None})
self.assertEqual(tagged_system_check.kwargs, {'app_configs': None, 'databases': None})
@override_system_checks([simple_system_check, tagged_system_check])
def test_invalid_tag(self):