[Django] #33140: order of checks

2 views
Skip to first unread message

Django

unread,
Sep 24, 2021, 3:18:12 PM9/24/21
to django-...@googlegroups.com
#33140: order of checks
-----------------------------------------+------------------------
Reporter: David Szotten | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 3.2
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 |
-----------------------------------------+------------------------
https://github.com/django/django/pull/8967 changes the checks registry to
use sets instead of lists. i may be missing it, but i can't see a ticket
reference or any reason for the change

i just had a bug where the behaviour of a generated file (happened to be
by https://github.com/adenh93/django-typomatic) depends on collecting
classes, which in turn depends on the import order of files in my project

i had two different invocations get different orderings of checks causing
different import orders

could we consider switching back to lists? or e.g. using `sorted` instead
of `list` in `get_checks`
https://github.com/django/django/blob/8036b53de61f16098a2f22c50621987182bdfaed/django/core/checks/registry.py#L96

--
Ticket URL: <https://code.djangoproject.com/ticket/33140>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Sep 28, 2021, 2:36:23 AM9/28/21
to django-...@googlegroups.com
#33140: Enforce ordering of checks.
-------------------------------+--------------------------------------

Reporter: David Szotten | Owner: nobody
Type: New feature | Status: closed
Component: Core (Other) | Version: 3.2
Severity: Normal | Resolution: wontfix

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by Carlton Gibson):

* status: new => closed
* resolution: => wontfix
* type: Uncategorized => New feature
* component: Uncategorized => Core (Other)


Comment:

Hi David.

I don't think it's really part of the System Check API/contract that
they're run in any order. I don't think making it such is a good idea.
(Indeed it might be a feature that they're not...) We could throw in the
`sorted` — what would it hurt? — but then folks would end up
using/depending on it. As such I'm inclined to `wontfix`.

The change from the PR was so long ago now I can't see a realistic case
for arguing there's a regression here. A set was the right collection type
all along.

So to your problem, I think a change in `django-typomatic` is the way to
go. Even if we accepted this here it would be roughly a year before it was
isn an available version of Django. Applying some kind of sort in `django-
typomatic`, on the other hand, could be available ≈immediately, and it's
probably something it should do anyway…

--
Ticket URL: <https://code.djangoproject.com/ticket/33140#comment:1>

Django

unread,
Sep 28, 2021, 4:28:48 PM9/28/21
to django-...@googlegroups.com
#33140: Enforce ordering of checks.
-------------------------------+--------------------------------------
Reporter: David Szotten | Owner: nobody
Type: New feature | Status: new

Component: Core (Other) | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by David Szotten):

* status: closed => new
* resolution: wontfix =>


Comment:

to be clear, i wasn't suggesting guaranteeing any _particular order, but
rather making sure the order is consistent between invocations and systems

django-typomatic was my example (and i fixed my issue by disabling the
checks for the management command in question), but the general problem is
hard-to-track-down bugs caused by different import orders on different
systems (in my case i have yet to figure out what even _caused the
iteration order to change)

(happy for you to re-close if you still think that's the best action here,
but wanted to clarify my suggestion)

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

Django

unread,
Sep 29, 2021, 5:25:08 AM9/29/21
to django-...@googlegroups.com
#33140: Enforce ordering of checks.
-------------------------------+--------------------------------------
Reporter: David Szotten | Owner: nobody
Type: New feature | Status: closed

Component: Core (Other) | Version: 3.2
Severity: Normal | Resolution: wontfix

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by Carlton Gibson):

* status: new => closed
* resolution: => wontfix


Comment:

Hey David. I'll reclose since it's best to take these kind of discussions
to the DevelopersMailingList (as per the
[https://docs.djangoproject.com/en/3.2/internals/contributing/triaging-
tickets#s-id2 triage guidelines for wontfix tickets])

My thought was that really System Checks should be able to run in any
order — like `--random` for a test runner — that was my point about
''Indeed it might be a feature...''. Maybe that's not right though, so
seeing what a wider audience think may be worthwhile.

> ...bugs caused by different import orders ...

I don't know but, my thought here is that ordering the system checks is at
best a band-aid over a deeper problem here. Import order isn't really
something that's under user-control. Relying on it seems to be asking for
trouble... — but maybe others can speak differently.

--
Ticket URL: <https://code.djangoproject.com/ticket/33140#comment:3>

Reply all
Reply to author
Forward
0 new messages