Browse Source

Fixed #31546 -- Allowed specifying list of tags in Command.requires_system_checks.

Hasan Ramezani 4 years ago
parent
commit
c60524c658
30 changed files with 156 additions and 41 deletions
  1. 1 1
      django/contrib/auth/management/commands/changepassword.py
  2. 1 1
      django/contrib/gis/management/commands/ogrinspect.py
  3. 1 8
      django/contrib/staticfiles/management/commands/collectstatic.py
  4. 27 4
      django/core/management/base.py
  5. 1 1
      django/core/management/commands/check.py
  6. 1 1
      django/core/management/commands/compilemessages.py
  7. 1 1
      django/core/management/commands/createcachetable.py
  8. 1 1
      django/core/management/commands/dbshell.py
  9. 1 1
      django/core/management/commands/diffsettings.py
  10. 1 1
      django/core/management/commands/inspectdb.py
  11. 1 1
      django/core/management/commands/makemessages.py
  12. 1 1
      django/core/management/commands/migrate.py
  13. 1 1
      django/core/management/commands/runserver.py
  14. 1 1
      django/core/management/commands/shell.py
  15. 1 1
      django/core/management/commands/test.py
  16. 1 1
      django/core/management/commands/testserver.py
  17. 1 1
      django/core/management/templates.py
  18. 9 2
      docs/howto/custom-management-commands.txt
  19. 2 0
      docs/internals/deprecation.txt
  20. 1 1
      docs/ref/django-admin.txt
  21. 9 0
      docs/releases/3.2.txt
  22. 1 1
      tests/admin_scripts/management/commands/app_command.py
  23. 1 1
      tests/admin_scripts/management/commands/base_command.py
  24. 1 1
      tests/admin_scripts/management/commands/label_command.py
  25. 1 1
      tests/admin_scripts/management/commands/noargs_command.py
  26. 3 3
      tests/admin_scripts/tests.py
  27. 1 1
      tests/user_commands/management/commands/dance.py
  28. 8 0
      tests/user_commands/management/commands/no_system_checks.py
  29. 9 0
      tests/user_commands/management/commands/specific_system_checks.py
  30. 67 3
      tests/user_commands/tests.py

+ 1 - 1
django/contrib/auth/management/commands/changepassword.py

@@ -12,7 +12,7 @@ UserModel = get_user_model()
 class Command(BaseCommand):
     help = "Change a user's password for django.contrib.auth."
     requires_migrations_checks = True
-    requires_system_checks = False
+    requires_system_checks = []
 
     def _get_pass(self, prompt="Password: "):
         p = getpass.getpass(prompt=prompt)

+ 1 - 1
django/contrib/gis/management/commands/ogrinspect.py

@@ -37,7 +37,7 @@ class Command(BaseCommand):
         ' ./manage.py ogrinspect zipcode.shp Zipcode'
     )
 
-    requires_system_checks = False
+    requires_system_checks = []
 
     def add_arguments(self, parser):
         parser.add_argument('data_source', help='Path to the data source.')

+ 1 - 8
django/contrib/staticfiles/management/commands/collectstatic.py

@@ -16,7 +16,7 @@ class Command(BaseCommand):
     settings.STATIC_ROOT.
     """
     help = "Collect static files in a single location."
-    requires_system_checks = False
+    requires_system_checks = [Tags.staticfiles]
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
@@ -36,10 +36,6 @@ class Command(BaseCommand):
         return True
 
     def add_arguments(self, parser):
-        parser.add_argument(
-            '--skip-checks', action='store_true',
-            help='Skip system checks.',
-        )
         parser.add_argument(
             '--noinput', '--no-input', action='store_false', dest='interactive',
             help="Do NOT prompt the user for input of any kind.",
@@ -151,9 +147,6 @@ class Command(BaseCommand):
 
     def handle(self, **options):
         self.set_options(**options)
-        if not options['skip_checks']:
-            self.check(tags=[Tags.staticfiles])
-
         message = ['\n']
         if self.dry_run:
             message.append(

+ 27 - 4
django/core/management/base.py

@@ -4,6 +4,7 @@ be executed through ``django-admin`` or ``manage.py``).
 """
 import os
 import sys
+import warnings
 from argparse import ArgumentParser, HelpFormatter
 from io import TextIOBase
 
@@ -12,6 +13,9 @@ from django.core import checks
 from django.core.exceptions import ImproperlyConfigured
 from django.core.management.color import color_style, no_style
 from django.db import DEFAULT_DB_ALIAS, connections
+from django.utils.deprecation import RemovedInDjango41Warning
+
+ALL_CHECKS = '__all__'
 
 
 class CommandError(Exception):
@@ -203,8 +207,11 @@ class BaseCommand:
         migrations on disk don't match the migrations in the database.
 
     ``requires_system_checks``
-        A boolean; if ``True``, entire Django project will be checked for errors
-        prior to executing the command. Default value is ``True``.
+        A list or tuple of tags, e.g. [Tags.staticfiles, Tags.models]. System
+        checks registered in the chosen tags will be checked for errors prior
+        to executing the command. The value '__all__' can be used to specify
+        that all system checks should be performed. Default value is '__all__'.
+
         To validate an individual application's models
         rather than all applications' models, call
         ``self.check(app_configs)`` from ``handle()``, where ``app_configs``
@@ -222,7 +229,7 @@ class BaseCommand:
     _called_from_command_line = False
     output_transaction = False  # Whether to wrap the output in a "BEGIN; COMMIT;"
     requires_migrations_checks = False
-    requires_system_checks = True
+    requires_system_checks = '__all__'
     # Arguments, common to all commands, which aren't defined by the argument
     # parser.
     base_stealth_options = ('stderr', 'stdout')
@@ -239,6 +246,19 @@ class BaseCommand:
         else:
             self.style = color_style(force_color)
             self.stderr.style_func = self.style.ERROR
+        if self.requires_system_checks in [False, True]:
+            warnings.warn(
+                "Using a boolean value for requires_system_checks is "
+                "deprecated. Use '__all__' instead of True, and [] (an empty "
+                "list) instead of False.",
+                RemovedInDjango41Warning,
+            )
+            self.requires_system_checks = ALL_CHECKS if self.requires_system_checks else []
+        if (
+            not isinstance(self.requires_system_checks, (list, tuple)) and
+            self.requires_system_checks != ALL_CHECKS
+        ):
+            raise TypeError('requires_system_checks must be a list or tuple.')
 
     def get_version(self):
         """
@@ -365,7 +385,10 @@ class BaseCommand:
             self.stderr = OutputWrapper(options['stderr'])
 
         if self.requires_system_checks and not options['skip_checks']:
-            self.check()
+            if self.requires_system_checks == ALL_CHECKS:
+                self.check()
+            else:
+                self.check(tags=self.requires_system_checks)
         if self.requires_migrations_checks:
             self.check_migrations()
         output = self.handle(*args, **options)

+ 1 - 1
django/core/management/commands/check.py

@@ -7,7 +7,7 @@ from django.core.management.base import BaseCommand, CommandError
 class Command(BaseCommand):
     help = "Checks the entire Django project for potential problems."
 
-    requires_system_checks = False
+    requires_system_checks = []
 
     def add_arguments(self, parser):
         parser.add_argument('args', metavar='app_label', nargs='*')

+ 1 - 1
django/core/management/commands/compilemessages.py

@@ -29,7 +29,7 @@ def is_writable(path):
 class Command(BaseCommand):
     help = 'Compiles .po files to .mo files for use with builtin gettext support.'
 
-    requires_system_checks = False
+    requires_system_checks = []
 
     program = 'msgfmt'
     program_options = ['--check-format']

+ 1 - 1
django/core/management/commands/createcachetable.py

@@ -10,7 +10,7 @@ from django.db import (
 class Command(BaseCommand):
     help = "Creates the tables needed to use the SQL cache backend."
 
-    requires_system_checks = False
+    requires_system_checks = []
 
     def add_arguments(self, parser):
         parser.add_argument(

+ 1 - 1
django/core/management/commands/dbshell.py

@@ -10,7 +10,7 @@ class Command(BaseCommand):
         "default database if none is provided."
     )
 
-    requires_system_checks = False
+    requires_system_checks = []
 
     def add_arguments(self, parser):
         parser.add_argument(

+ 1 - 1
django/core/management/commands/diffsettings.py

@@ -10,7 +10,7 @@ class Command(BaseCommand):
     help = """Displays differences between the current settings.py and Django's
     default settings."""
 
-    requires_system_checks = False
+    requires_system_checks = []
 
     def add_arguments(self, parser):
         parser.add_argument(

+ 1 - 1
django/core/management/commands/inspectdb.py

@@ -8,7 +8,7 @@ from django.db.models.constants import LOOKUP_SEP
 
 class Command(BaseCommand):
     help = "Introspects the database tables in the given database and outputs a Django model module."
-    requires_system_checks = False
+    requires_system_checks = []
     stealth_options = ('table_name_filter',)
     db_module = 'django.db'
 

+ 1 - 1
django/core/management/commands/makemessages.py

@@ -206,7 +206,7 @@ class Command(BaseCommand):
     translatable_file_class = TranslatableFile
     build_file_class = BuildFile
 
-    requires_system_checks = False
+    requires_system_checks = []
 
     msgmerge_options = ['-q', '--previous']
     msguniq_options = ['--to-code=utf-8']

+ 1 - 1
django/core/management/commands/migrate.py

@@ -20,7 +20,7 @@ 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
+    requires_system_checks = []
 
     def add_arguments(self, parser):
         parser.add_argument(

+ 1 - 1
django/core/management/commands/runserver.py

@@ -25,7 +25,7 @@ class Command(BaseCommand):
     help = "Starts a lightweight Web server for development."
 
     # Validation is called explicitly each time the server is reloaded.
-    requires_system_checks = False
+    requires_system_checks = []
     stealth_options = ('shutdown_message',)
 
     default_addr = '127.0.0.1'

+ 1 - 1
django/core/management/commands/shell.py

@@ -14,7 +14,7 @@ class Command(BaseCommand):
         "as code."
     )
 
-    requires_system_checks = False
+    requires_system_checks = []
     shells = ['ipython', 'bpython', 'python']
 
     def add_arguments(self, parser):

+ 1 - 1
django/core/management/commands/test.py

@@ -10,7 +10,7 @@ class Command(BaseCommand):
     help = 'Discover and run tests in the specified modules or the current directory.'
 
     # DiscoverRunner runs the checks after databases are set up.
-    requires_system_checks = False
+    requires_system_checks = []
     test_runner = None
 
     def run_from_argv(self, argv):

+ 1 - 1
django/core/management/commands/testserver.py

@@ -6,7 +6,7 @@ from django.db import connection
 class Command(BaseCommand):
     help = 'Runs a development server with data from the given fixture(s).'
 
-    requires_system_checks = False
+    requires_system_checks = []
 
     def add_arguments(self, parser):
         parser.add_argument(

+ 1 - 1
django/core/management/templates.py

@@ -28,7 +28,7 @@ class TemplateCommand(BaseCommand):
     :param directory: The directory to which the template should be copied.
     :param options: The additional variables passed to project or app templates
     """
-    requires_system_checks = False
+    requires_system_checks = []
     # The supported URL schemes
     url_schemes = ['http', 'https', 'ftp']
     # Rewrite the following suffixes when determining the target filename.

+ 9 - 2
docs/howto/custom-management-commands.txt

@@ -215,8 +215,15 @@ All attributes can be set in your derived class and can be used in
 
 .. attribute:: BaseCommand.requires_system_checks
 
-    A boolean; if ``True``, the entire Django project will be checked for
-    potential problems prior to executing the command. Default value is ``True``.
+    A list or tuple of tags, e.g. ``[Tags.staticfiles, Tags.models]``. System
+    checks registered in the chosen tags will be checked for errors prior to
+    executing the command. The value ``'__all__'`` can be used to specify
+    that all system checks should be performed. Default value is ``'__all__'``.
+
+    .. versionchanged:: 3.2
+
+        In older versions, the ``requires_system_checks`` attribute expects a
+        boolean value instead of a list or tuple of tags.
 
 .. attribute:: BaseCommand.style
 

+ 2 - 0
docs/internals/deprecation.txt

@@ -19,6 +19,8 @@ details on these changes.
   ``copy.deepcopy()`` to class attributes in ``TestCase.setUpTestData()`` will
   be removed.
 
+* ``BaseCommand.requires_system_checks`` won't support boolean values.
+
 .. _deprecation-removed-in-4.0:
 
 4.0

+ 1 - 1
docs/ref/django-admin.txt

@@ -1826,7 +1826,7 @@ colored output to another command.
 Skips running system checks prior to running the command. This option is only
 available if the
 :attr:`~django.core.management.BaseCommand.requires_system_checks` command
-attribute is set to ``True``.
+attribute is not an empty list or tuple.
 
 Example usage::
 

+ 9 - 0
docs/releases/3.2.txt

@@ -161,6 +161,11 @@ Management Commands
   connection. In that case, check for a consistent migration history is
   skipped.
 
+* :attr:`.BaseCommand.requires_system_checks` now supports specifying a list of
+  tags. System checks registered in the chosen tags will be checked for errors
+  prior to executing the command. In previous versions, either all or none
+  of the system checks were performed.
+
 Migrations
 ~~~~~~~~~~
 
@@ -273,3 +278,7 @@ Miscellaneous
 * Assigning objects which don't support creating deep copies with
   :py:func:`copy.deepcopy` to class attributes in
   :meth:`.TestCase.setUpTestData` is deprecated.
+
+* Using a boolean value in :attr:`.BaseCommand.requires_system_checks` is
+  deprecated. Use ``'__all__'`` instead of ``True``, and ``[]`` (an empty list)
+  instead of ``False``.

+ 1 - 1
tests/admin_scripts/management/commands/app_command.py

@@ -3,7 +3,7 @@ from django.core.management.base import AppCommand
 
 class Command(AppCommand):
     help = 'Test Application-based commands'
-    requires_system_checks = False
+    requires_system_checks = []
 
     def handle_app_config(self, app_config, **options):
         print('EXECUTE:AppCommand name=%s, options=%s' % (app_config.name, sorted(options.items())))

+ 1 - 1
tests/admin_scripts/management/commands/base_command.py

@@ -3,7 +3,7 @@ from django.core.management.base import BaseCommand
 
 class Command(BaseCommand):
     help = 'Test basic commands'
-    requires_system_checks = False
+    requires_system_checks = []
 
     def add_arguments(self, parser):
         parser.add_argument('args', nargs='*')

+ 1 - 1
tests/admin_scripts/management/commands/label_command.py

@@ -3,7 +3,7 @@ from django.core.management.base import LabelCommand
 
 class Command(LabelCommand):
     help = "Test Label-based commands"
-    requires_system_checks = False
+    requires_system_checks = []
 
     def handle_label(self, label, **options):
         print('EXECUTE:LabelCommand label=%s, options=%s' % (label, sorted(options.items())))

+ 1 - 1
tests/admin_scripts/management/commands/noargs_command.py

@@ -3,7 +3,7 @@ from django.core.management.base import BaseCommand
 
 class Command(BaseCommand):
     help = "Test No-args commands"
-    requires_system_checks = False
+    requires_system_checks = []
 
     def handle(self, **options):
         print('EXECUTE: noargs_command options=%s' % sorted(options.items()))

+ 3 - 3
tests/admin_scripts/tests.py

@@ -1395,7 +1395,7 @@ class ManageTestserver(SimpleTestCase):
 # the commands are correctly parsed and processed.
 ##########################################################################
 class ColorCommand(BaseCommand):
-    requires_system_checks = False
+    requires_system_checks = []
 
     def handle(self, *args, **options):
         self.stdout.write('Hello, world!', self.style.ERROR)
@@ -1541,7 +1541,7 @@ class CommandTypes(AdminScriptTestCase):
 
     def test_custom_stdout(self):
         class Command(BaseCommand):
-            requires_system_checks = False
+            requires_system_checks = []
 
             def handle(self, *args, **options):
                 self.stdout.write("Hello, World!")
@@ -1558,7 +1558,7 @@ class CommandTypes(AdminScriptTestCase):
 
     def test_custom_stderr(self):
         class Command(BaseCommand):
-            requires_system_checks = False
+            requires_system_checks = []
 
             def handle(self, *args, **options):
                 self.stderr.write("Hello, World!")

+ 1 - 1
tests/user_commands/management/commands/dance.py

@@ -4,7 +4,7 @@ from django.core.management.base import BaseCommand, CommandError
 class Command(BaseCommand):
     help = "Dance around like a madman."
     args = ''
-    requires_system_checks = True
+    requires_system_checks = '__all__'
 
     def add_arguments(self, parser):
         parser.add_argument("integer", nargs='?', type=int, default=0)

+ 8 - 0
tests/user_commands/management/commands/no_system_checks.py

@@ -0,0 +1,8 @@
+from django.core.management.base import BaseCommand
+
+
+class Command(BaseCommand):
+    requires_system_checks = []
+
+    def handle(self, *args, **options):
+        pass

+ 9 - 0
tests/user_commands/management/commands/specific_system_checks.py

@@ -0,0 +1,9 @@
+from django.core.checks import Tags
+from django.core.management.base import BaseCommand
+
+
+class Command(BaseCommand):
+    requires_system_checks = [Tags.staticfiles, Tags.models]
+
+    def handle(self, *args, **options):
+        pass

+ 67 - 3
tests/user_commands/tests.py

@@ -6,6 +6,7 @@ from admin_scripts.tests import AdminScriptTestCase
 
 from django.apps import apps
 from django.core import management
+from django.core.checks import Tags
 from django.core.management import BaseCommand, CommandError, find_commands
 from django.core.management.utils import (
     find_command, get_random_secret_key, is_ignored_path,
@@ -13,8 +14,9 @@ from django.core.management.utils import (
 )
 from django.db import connection
 from django.test import SimpleTestCase, override_settings
-from django.test.utils import captured_stderr, extend_sys_path
+from django.test.utils import captured_stderr, extend_sys_path, ignore_warnings
 from django.utils import translation
+from django.utils.deprecation import RemovedInDjango41Warning
 from django.utils.version import PY37
 
 from .management.commands import dance
@@ -59,13 +61,13 @@ class CommandTests(SimpleTestCase):
         with self.assertRaises(CommandError) as cm:
             management.call_command('dance', example="raise")
         self.assertEqual(cm.exception.returncode, 3)
-        dance.Command.requires_system_checks = False
+        dance.Command.requires_system_checks = []
         try:
             with captured_stderr() as stderr, self.assertRaises(SystemExit) as cm:
                 management.ManagementUtility(['manage.py', 'dance', '--example=raise']).execute()
             self.assertEqual(cm.exception.code, 3)
         finally:
-            dance.Command.requires_system_checks = True
+            dance.Command.requires_system_checks = '__all__'
         self.assertIn("CommandError", stderr.getvalue())
 
     def test_no_translations_deactivate_translations(self):
@@ -155,6 +157,7 @@ class CommandTests(SimpleTestCase):
 
         def patched_check(self_, **kwargs):
             self.counter += 1
+            self.kwargs = kwargs
 
         saved_check = BaseCommand.check
         BaseCommand.check = patched_check
@@ -163,9 +166,28 @@ class CommandTests(SimpleTestCase):
             self.assertEqual(self.counter, 0)
             management.call_command("dance", verbosity=0, skip_checks=False)
             self.assertEqual(self.counter, 1)
+            self.assertEqual(self.kwargs, {})
         finally:
             BaseCommand.check = saved_check
 
+    def test_requires_system_checks_empty(self):
+        with mock.patch('django.core.management.base.BaseCommand.check') as mocked_check:
+            management.call_command('no_system_checks')
+        self.assertIs(mocked_check.called, False)
+
+    def test_requires_system_checks_specific(self):
+        with mock.patch('django.core.management.base.BaseCommand.check') as mocked_check:
+            management.call_command('specific_system_checks')
+        mocked_check.called_once_with(tags=[Tags.staticfiles, Tags.models])
+
+    def test_requires_system_checks_invalid(self):
+        class Command(BaseCommand):
+            requires_system_checks = 'x'
+
+        msg = 'requires_system_checks must be a list or tuple.'
+        with self.assertRaisesMessage(TypeError, msg):
+            Command()
+
     def test_check_migrations(self):
         requires_migrations_checks = dance.Command.requires_migrations_checks
         self.assertIs(requires_migrations_checks, False)
@@ -334,3 +356,45 @@ class UtilsTests(SimpleTestCase):
     def test_normalize_path_patterns_truncates_wildcard_base(self):
         expected = [os.path.normcase(p) for p in ['foo/bar', 'bar/*/']]
         self.assertEqual(normalize_path_patterns(['foo/bar/*', 'bar/*/']), expected)
+
+
+class DeprecationTests(SimpleTestCase):
+    def test_requires_system_checks_warning(self):
+        class Command(BaseCommand):
+            pass
+
+        msg = (
+            "Using a boolean value for requires_system_checks is deprecated. "
+            "Use '__all__' instead of True, and [] (an empty list) instead of "
+            "False."
+        )
+        for value in [False, True]:
+            Command.requires_system_checks = value
+            with self.assertRaisesMessage(RemovedInDjango41Warning, msg):
+                Command()
+
+    @ignore_warnings(category=RemovedInDjango41Warning)
+    def test_requires_system_checks_true(self):
+        class Command(BaseCommand):
+            requires_system_checks = True
+
+            def handle(self, *args, **options):
+                pass
+
+        command = Command()
+        with mock.patch('django.core.management.base.BaseCommand.check') as mocked_check:
+            management.call_command(command, skip_checks=False)
+        mocked_check.assert_called_once_with()
+
+    @ignore_warnings(category=RemovedInDjango41Warning)
+    def test_requires_system_checks_false(self):
+        class Command(BaseCommand):
+            requires_system_checks = False
+
+            def handle(self, *args, **options):
+                pass
+
+        command = Command()
+        with mock.patch('django.core.management.base.BaseCommand.check') as mocked_check:
+            management.call_command(command)
+        self.assertIs(mocked_check.called, False)