[Django] #36663: Make management commands default to running checks against all databases

[Django]

#36663: Make management commands default to running checks against all databases
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Type: Bug
Status: new | Component: Core
| (System checks)
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
In 0b83c8cc4db95812f1e15ca19d78614e94cf38dd (refs #31055), many field
checks stopped running by default (see ticket:31286#comment:10 for a long
analysis), thus postponing warnings and errors that would be detected
`runserver` or `makemigrations` until `migrate`.

As proposed by Simon in ticket:31286#comment:9, the running of these
checks should be restored.
--
Ticket URL: <https://code.djangoproject.com/ticket/36663>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36663: Make management commands default to running checks against all databases
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: (none)
Type: Bug | Status: new
Component: Core (System | Version: dev
checks) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 1

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Tim Graham):

* has_patch: 0 => 1
* needs_docs: 0 => 1

Comment:

[https://github.com/django/django/pull/19912 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/36663#comment:1>

[Django]

#36663: Make management commands default to running checks against all databases

--------------------------------------+------------------------------------

Reporter: Tim Graham | Owner: (none)
Type: Bug | Status: new

Component: Core (System checks) | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Simon Charette):

* cc: Simon Charette (added)
* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/36663#comment:2>

[Django]

#36663: Make management commands default to running checks against all databases
--------------------------------------+------------------------------------
Reporter: Tim Graham | Owner: (none)
Type: Bug | Status: new
Component: Core (System checks) | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Comment (by Tim Graham):

Simon, do you feel we shoul try to maintain the behavior: "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 `migrate` command or if you
specify configured database aliases using the `--database` option when
calling the `check` command."

I guess we would have to go back to detecting whether or not they
requested via `--tag=database`.
--
Ticket URL: <https://code.djangoproject.com/ticket/36663#comment:3>

[Django]

#36663: Make management commands default to running checks against all databases

-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Varun
| Kasyap Pentamaraju
Type: Bug | Status: assigned

Component: Core (System | Version: dev
checks) |

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Varun Kasyap Pentamaraju):

* cc: Varun Kasyap Pentamaraju (added)
* owner: (none) => Varun Kasyap Pentamaraju
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36663#comment:4>

[Django]

#36663: Make management commands default to running checks against all databases
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: Simon
| Charette

Type: Bug | Status: assigned
Component: Core (System | Version: dev
checks) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* owner: Varun Kasyap Pentamaraju => Simon Charette

Comment:

Varun, notice this has only been in flight for 8 days and has recent
unanswered questions, it's not ready for the "vulture method" yet. Simon
can correct me if he does not wish to be the owner.
--
Ticket URL: <https://code.djangoproject.com/ticket/36663#comment:5>

[Django]

#36663: Make management commands default to running checks against all databases
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Core (System | Version: dev
checks) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

Thanks for the assignment Jacob and sorry for not doing it myself.

> Simon, do you feel we shoul try to maintain the behavior: "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 migrate command or
if you specify configured database aliases using the --database option
when calling the check command."

Tim, I don't have a strong opinion here, I'm slightly in favor of not
special casing database checks but I think it's a distinct problem that
would warrant a larger discussion. Do you have any objection on keeping
this ticket focused on making sure that when database checks are run they
default to all databases?

Any thoughts on the PR so far?
--
Ticket URL: <https://code.djangoproject.com/ticket/36663#comment:6>

[Django]

#36663: Make management commands default to running checks against all databases
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Core (System | Version: dev
checks) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham):

The proposed patch works for me.
--
Ticket URL: <https://code.djangoproject.com/ticket/36663#comment:7>

[Django]

#36663: Make management commands default to running checks against all databases
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Core (System | Version: dev
checks) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

You might have noticed it during the review Tim but the proposed patch
happens to achieve exactly what you're after.

Since the implicit exclusion of `Tags.databases`
[https://github.com/django/django/commit/0b83c8cc4db95812f1e15ca19d78614e94cf38dd
#diff-
f10ce154e1f5bcdba92b54ebf8cb57357b404a3f03ab51ca85a031358f7372ecL66-L69 is
no longer applied] all checks (including the `Tags.databases` ones) are
now performed on all management commands unless `--database` is specified.

A problem I see with this approach is that since system checks are
performed by management commands by default (unless they are marked with
`requires_system_checks = False`) it means that users who might be
interested achieving the previous behaviour cannot and must use `--skip-
checks` to disable all of them. This is fine by me but I wanted to flag it
for reviewers as I'll adjust the documentation accordingly.
--
Ticket URL: <https://code.djangoproject.com/ticket/36663#comment:8>

[Django]

#36663: Make management commands default to running checks against all databases
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Core (System | Version: dev
checks) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* needs_docs: 1 => 0

Comment:

Thinking more about this I ended up adjusting the PR slightly so it
continues to **not run database tagged checks by default** but passes
`databases` for all databases unless restricted with `--database` in
`check` and `migrate`. I extended our regression test coverage to make
sure these any change in this regards is explicit (as my initial draft
inadvertently changed that).

As discussed above changing how databases tagged checks should be run
should be discussed somewhere else as this ticket only ensures `databases`
passing works the way it should have when we implemented #31055.

Given the change is pretty internal and a bugfix I don't think it warrants
documentation but I'm happy to include some in the release notes if you
believe otherwise.
--
Ticket URL: <https://code.djangoproject.com/ticket/36663#comment:9>

[Django]

#36663: Make management commands default to running checks against all databases
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Core (System | Version: dev
checks) |
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/36663#comment:10>

[Django]

#36663: Make management commands default to running checks against all databases
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Simon
| Charette

Type: Bug | Status: closed

Component: Core (System | Version: dev
checks) |

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"3aba1fced8254435b947467739721ec6b4fb865c" 3aba1fce]:
{{{#!CommitTicketReference repository=""
revision="3aba1fced8254435b947467739721ec6b4fb865c"
Fixed #36663 -- Defaulted to running checks against all databases.

Regression in 0b83c8cc4db95812f1e15ca19d78614e94cf38dd which added
support for running checks only against databases but also defaulted to
running against no databases if not specified.

Note that this continues to *not* default to runing database tagged
checks for all management commands except the migrate one as whether or
not we should change this must be discussed further.

Thanks Tim Graham for surfacing how this was a problematic default.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36663#comment:11>

[Django]

#36663: Make management commands default to running checks against all databases
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Core (System | Version: dev
checks) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls <jacobtylerwalls@…>):

In [changeset:"14fb36e0b083ea963220602d01386cc0fb2c40e4" 14fb36e0]:
{{{#!CommitTicketReference repository=""
revision="14fb36e0b083ea963220602d01386cc0fb2c40e4"
Refs #36663, #36739 -- Added release note for running checks against all
databases.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36663#comment:12>