Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#31546 closed Cleanup/optimization (fixed)

Replace Command.requires_system_checks = True by something like Command.required_system_checks = '__all__'

Reported by: Hasan Ramezani Owned by: Hasan Ramezani
Component: Core (Management commands) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Created based on Simon ​comment on the PR

This makes me wonder if we want to replace Command.requires_system_checks = True by something like Command.required_system_checks = '__all__' that can be set to an empty list to achieve required_system_checks = False and allow subclasses that want only a subset to specify tags through it e.g. required_system_checks = ['staticfiles']. That would prevent having do to the manual and error prone options['skip_checks'] dance. In all cases that should be done in a different PR.

Change History (10)

comment:1 by Hasan Ramezani, 5 years ago

Owner: changed from nobody to Hasan Ramezani
Status: new → assigned
Type: Uncategorized → Cleanup/optimization

comment:2 by Carlton Gibson, 5 years ago

Triage Stage: Unreviewed → Accepted

OK, accepting based on the discussion on the PR. Thanks!

comment:3 by Hasan Ramezani, 5 years ago

I created a ​PR for start.
Should we go for deprecating the requires_system_checks = True/False?

in reply to:  3 comment:4 by Mariusz Felisiak, 5 years ago

Replying to Hasan Ramezani:

I created a ​PR for start.
Should we go for deprecating the requires_system_checks = True/False?

Yes.

comment:5 by Hasan Ramezani, 5 years ago

Has patch: set

comment:6 by Mariusz Felisiak, 5 years ago

Patch needs improvement: set

comment:7 by Mariusz Felisiak, 5 years ago

Patch needs improvement: unset
Triage Stage: Accepted → Ready for checkin

comment:8 by Carlton Gibson <carlton.gibson@…>, 5 years ago

Resolution: → fixed
Status: assigned → closed

In c60524c6:

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

comment:9 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In 1cb4950:

Refs #31546 -- Removed support for boolean values in Command.requires_system_checks.

Per deprecation timeline.

comment:10 by GitHub <noreply@…>, 3 years ago

In 829f4d1:

Refs #31546, Refs #34118 -- Corrected CommandTests.test_requires_system_checks_specific().

System checks are never called without skip_checks=False. Moreover,
called_once_with() is not a proper assertion and raise AttributeError
on Python 3.12.

Note: See TracTickets for help on using tickets.
Back to Top