#35920: Migrate command runs system checks regardless of the value of
requires_system_checks
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
| Walls
Type: Bug | Status: assigned
Component: Core (Management | Version: 5.1
commands) |
Severity: Normal | Resolution:
Keywords: skip-checks | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by Jacob Walls:
Old description:
> Without `--skip-checks=False`, the `migrate` command runs system checks
> regardless of the value of `requires_system_checks`, which is `[]` by
> [https://github.com/django/django/blob/4c452cc377f6f43acd90c6e54826ebd2e6219b0d/django/core/management/commands/migrate.py#L22
> default] (itself a little misleading) but ultimately something I would
> like to be able to override at the project level. The fact that `[]` is
> not respected and not overridable seems like a bug, and AFAIK it's an
> asymmetry with other commands.
>
> ----
>
> - I wrote a system check following the
> [https://docs.djangoproject.com/en/5.1/topics/checks/#field-model-
> manager-template-engine-and-database-checks documented example] inside
> Model.check().
> - I did this as part of a feature adding a new column to my model, and I
> queried this column in my check. I made the necessary migration.
> - During `manage.py migrate` my check ran before my column was added,
> raising `ProgrammingError`.
> - I verified `--skip-checks` works like a charm, but I don't want to
> impose cryptic failures on my fellow developers who expect plain
> `migrate` to work.
> - I attempted to override the `migrate` command in my project, like this:
>
> /app/management/commands/migrate.py
> {{{
> from django.core.checks.registry import registry
> from django.core.management.commands.migrate import Command as
> MigrateCommand
>
> class Command(MigrateCommand):
> # Silence model checks that may depend on new columns.
> requires_system_checks = list(registry.tags_available() - {"models"})
> }}}
>
> - Result: no change: `ProgrammingError`
>
> - Then, I added this patch to Django. (`--skip-checks` still works; here,
> I have to avoid it being added again):
> {{{#!diff
> diff --git a/django/core/management/commands/migrate.py
> b/django/core/management/commands/migrate.py
> index fa420ee6e3..4000d76f3b 100644
> --- a/django/core/management/commands/migrate.py
> +++ b/django/core/management/commands/migrate.py
> @@ -19,14 +19,9 @@ class Command(BaseCommand):
> help = (
> "Updates database schema. Manages both apps with migrations and
> those without."
> )
> - requires_system_checks = []
> + requires_system_checks = "__all__"
>
> def add_arguments(self, parser):
> - parser.add_argument(
> - "--skip-checks",
> - action="store_true",
> - help="Skip system checks.",
> - )
> parser.add_argument(
> "app_label",
> nargs="?",
> @@ -99,7 +94,7 @@ class Command(BaseCommand):
> def handle(self, *args, **options):
> database = options["database"]
> if not options["skip_checks"]:
> - self.check(databases=[database])
> + self.check(tags=self.requires_system_checks,
> databases=[database])
>
> self.verbosity = options["verbosity"]
> self.interactive = options["interactive"]
>
> }}}
>
> - Result: check skipped as expected via my project-level override of the
> migrate command.
>
> ----
>
> I suppose there's another question here about whether `Tags.models`
> checks should run as part of the migrate command, or whether the change I
> made at the project level should be pulled into core, given that this is
> a reasonably realistic scenario for development? I can take that to the
> forum later. But at the moment, I'm just hoping to get the override
> working. Happy to PR this if welcome :-)
New description:
Without `--skip-checks=False`, the `migrate` command runs system checks
regardless of the value of `requires_system_checks`, which is `[]` by
[https://github.com/django/django/blob/4c452cc377f6f43acd90c6e54826ebd2e6219b0d/django/core/management/commands/migrate.py#L22
default] (itself a little misleading) but ultimately something I would
like to be able to override at the project level. The fact that `[]` is
not respected and not overridable seems like a bug, and AFAIK it's an
asymmetry with other commands. (EDIT: turns out, `runserver` behaves in a
similar way, with a
[https://github.com/django/django/blob/c2c7dbb2f88ce8f0ef6d48a61b93866c9926349a/django/core/management/commands/runserver.py#L30
comment from 17 years ago], predating migrations, explaining there is a
separate mechanism from the one that became `requires_system_checks` that
runs the system checks. I think we can improve the story here.)
----
- I wrote a system check following the
[https://docs.djangoproject.com/en/5.1/topics/checks/#field-model-manager-
template-engine-and-database-checks documented example] inside
Model.check().
- I did this as part of a feature adding a new column to my model, and I
queried this column in my check. I made the necessary migration.
- During `manage.py migrate` my check ran before my column was added,
raising `ProgrammingError`.
- I verified `--skip-checks` works like a charm, but I don't want to
impose cryptic failures on my fellow developers who expect plain `migrate`
to work.
- I attempted to override the `migrate` command in my project, like this:
/app/management/commands/migrate.py
{{{
from django.core.checks.registry import registry
from django.core.management.commands.migrate import Command as
MigrateCommand
class Command(MigrateCommand):
# Silence model checks that may depend on new columns.
requires_system_checks = list(registry.tags_available() - {"models"})
}}}
- Result: no change: `ProgrammingError`
- Then, I added this patch to Django. (`--skip-checks` still works; here,
I have to avoid it being added again):
{{{#!diff
diff --git a/django/core/management/commands/migrate.py
b/django/core/management/commands/migrate.py
index fa420ee6e3..4000d76f3b 100644
--- a/django/core/management/commands/migrate.py
+++ b/django/core/management/commands/migrate.py
@@ -19,14 +19,9 @@ class Command(BaseCommand):
help = (
"Updates database schema. Manages both apps with migrations and
those without."
)
- requires_system_checks = []
+ requires_system_checks = "__all__"
def add_arguments(self, parser):
- parser.add_argument(
- "--skip-checks",
- action="store_true",
- help="Skip system checks.",
- )
parser.add_argument(
"app_label",
nargs="?",
@@ -99,7 +94,7 @@ class Command(BaseCommand):
def handle(self, *args, **options):
database = options["database"]
if not options["skip_checks"]:
- self.check(databases=[database])
+ self.check(tags=self.requires_system_checks,
databases=[database])
self.verbosity = options["verbosity"]
self.interactive = options["interactive"]
}}}
- Result: check skipped as expected via my project-level override of the
migrate command.
----
I suppose there's another question here about whether `Tags.models` checks
should run as part of the migrate command, or whether the change I made at
the project level should be pulled into core, given that this is a
reasonably realistic scenario for development? I can take that to the
forum later. But at the moment, I'm just hoping to get the override
working. Happy to PR this if welcome :-)
--
--
Ticket URL: <https://code.djangoproject.com/ticket/35920#comment:1>
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/0107019347409650-a7f314ac-258d-43e5-8aa6-ef6e04d5278d-000000%40eu-central-1.amazonses.com.
