Opened 3 days ago

Last modified 15 hours ago

#36273 assigned Cleanup/optimization

Move Index system checks from Model to Index.check()

Reported by: Tim Graham Owned by: Tanishq
Component: Core (System checks) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In #30613 (53209f78302a639032afabf5326d28d4ddd9d03c), some Index validation was moved from Index.__init__() to Model.check(). This prevents Index subclasses from customizing or extending the validation. The logic should be in Index.check(), following the pattern of fields, constraints, etc.

According to the ticket's flags, the next step(s) to move this issue forward are:

  • To provide a patch by sending a pull request. Claim the ticket when you start working so that someone else doesn't duplicate effort. Before sending a pull request, review your work against the patch review checklist. Check the "Has patch" flag on the ticket after sending a pull request and include a link to the pull request in the ticket comment when making that update. The usual format is: [https://github.com/django/django/pull/#### PR].

Change History (5)

comment:1 by Simon Charette, 3 days ago

Triage Stage: Unreviewed → Accepted

Agreed, this is coherent with #35234 (0fb104dda287431f5ab74532e45e8471e22b58c8) to move constraint checks to Constraint.check.

comment:2 by Tanishq, 37 hours ago

@Simon Charette

Hi,

I'd like to claim ticket #36273 and work on moving the Index system checks from Model.check() to Index.check(), following the pattern used for fields and constraints. I'll submit a PR once the changes are ready.

Let me know if there are any specific considerations or related discussions I should be aware of before proceeding.

comment:3 by Simon Charette, 34 hours ago

The ticket is yours Tanishq, make sure to assign it to you.

As mentioned above the required changes can borrow a lot from 0fb104dda287431f5ab74532e45e8471e22b58c8 except for the fact that we can name the method Index.check from the get go (there's no conflict with a pre-existing attribute).

comment:4 by Simon Charette, 20 hours ago

Small note that this work landing would make implementing ticket:32770#comment:5 way more straightforward.

comment:5 by Tanishq, 15 hours ago

Owner: set to Tanishq
Status: new → assigned
Note: See TracTickets for help on using tickets.
Back to Top