[Django] #29799: Allow registration and unregistration of lookups per field instances.
Django
-------------------------------------+-------------------------------------
Reporter: Simon | Owner: nobody
Charette |
Type: New | Status: new
feature |
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Under certain circumstances it might be useful to register or unregister
model field lookups on a per instance basis instead of a per class basis.
For example, it could be useful to disable costly lookups such as
`contains` and `icontains` on some large table model `CharField`s without
defining them using a `CharField` subclass to prevent them from being used
in production. Similarly you might want to register or override a specific
lookup for a third party app model field that doesn't allow swapping.
This was initially mentioned in #16187 which introduced this new lookup
registration system.
https://code.djangoproject.com/ticket/16187#comment:14
> Having the ability to hook a custom lookup to a given field instance
would be really nice to have...
https://code.djangoproject.com/ticket/16187#comment:22
> Add a way to register lookups to fields (I think either instances or
classes is wanted behaviour).
But not discussed afterwhile. I believe class based registration was
chosen because it was easier to implement but I agree that being able to
register custom lookups per field instance would be good feature addition.
--
Ticket URL: <https://code.djangoproject.com/ticket/29799>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
Yes. Super.
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:1>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Sergey Fedoseev (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:2>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Can Sarıgöl (added)
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/11408 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:3>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_docs: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:4>
Django
-------------------------------------+-------------------------------------
| Sarıgöl
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => Can Sarıgöl
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:5>
Django
-------------------------------------+-------------------------------------
| Sarıgöl
Type: New feature | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Himanshu Balasamanta):
@carltongibson , @charettes . I wanted to clarify exactly what all the
scope of the ticket covers.
As per my understanding,
- Enable registration and remove a lookup for a particular instance as
opposed to a class.
- Enable lookups registered for the class to be unregistered (and maybe
re-registered?) for instances of that class.
Description of the functions would be as follows
- get_lookups()
- when called by a field instance, returns all class and instance
lookups currently registered for that field instance
- when called by a class, returns all class lookups currently registered
for that class
- register_lookup
- when called by a field instance, registers the lookup for that field
instance
- when called by a class, registers the lookup for that class
- unregister_lookup
1. when called by a field instance,
- if the lookup is already registered with the instance, it gets
removed,
- if the lookup is registered with the instance class, that particular
instance is exempted
- else throws error
2. when called by a class, if the lookup is already registered with the
class, it is removed. Else error is thrown.
Doubts:
1. Should class lookups that are unregistered for an instance be re-
registered using the register_lookup method itself?
2. Should there be a method to specifically get lookups that were
registered for the instance and not the class?
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:6>
Django
-------------------------------------+-------------------------------------
| Sarıgöl
Type: New feature | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Sergey Fedoseev (removed)
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:7>
Django
-------------------------------------+-------------------------------------
| Sarıgöl
Type: New feature | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Carlton Gibson):
Hi Himanshu.
The rough outline looks right. There are various variations but, ''I'd
like to add a custom lookup just for **this** model'' — the model being
the bearer of field instances; ''I'd like to disable **this** lookup on
this model'', and so on.
The specific behaviours are hard to say definitively one way or the other
until we get to the review stage. Looking at test cases, and seeing how
the API looks at that point is much easier to speak clearly to.
(For example, I'm not sure what to say about trying to unregister a non-
registered lookup on a class... — we **could** error, or we could `pass`:
the end effect is that the lookup is not registered on the class... 🤔 —
it's the kind of detail we can settle with multiple sets of eyes at review
time.)
I know you're looking at this targeting GSoC, so for the the sake of a
proposal, noting the areas to be decided is OK. Showing that you've
considered points already raised is important — make sure you looked at
[https://github.com/django/django/pull/15529#issuecomment-1073412321
Simon's comment on your draft PR] — I'm sure you did. Plus #33626 and the
PR there you've made are all good signs — that's on my list to review next
week.
I hope that helps. 🙂
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:8>
Django
-------------------------------------+-------------------------------------
| Sarıgöl
Type: New feature | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Himanshu Balasamanta):
Replying to [comment:8 Carlton Gibson]:
> The rough outline looks right. There are various variations but, ''I'd
like to add a custom lookup just for **this** model'' — the model being
the bearer of field instances; ''I'd like to disable **this** lookup on
this model'', and so on.
Agreed.👍 It would be more user friendly.
So, along with register_lookup(self,lookup,lookup_name), I shall add a
Model.register_lookup(field_name : str, lookup : Lookup, lookup_name :
str) too.
> I know you're looking at this targeting GSoC, so for the the sake of a
proposal, noting the areas to be decided is OK. Showing that you've
considered points already raised is important — make sure you looked at
[https://github.com/django/django/pull/15529#issuecomment-1073412321
Simon's comment on your draft PR] — I'm sure you did. Plus #33626 and the
PR there you've made are all good signs — that's on my list to review next
week.
>
> I hope that helps. 🙂
>
>
Thank You!
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:9>
Django
-------------------------------------+-------------------------------------
| Sarıgöl
Type: New feature | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Himanshu Balasamanta (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:10>
Django
-------------------------------------+-------------------------------------
| AllenJonathan
Type: New feature | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Simon Charette (added)
* needs_better_patch: 1 => 0
* owner: Can Sarıgöl => AllenJonathan
* keywords: => GSoC
* needs_docs: 1 => 0
* has_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:11>
Django
-------------------------------------+-------------------------------------
| AllenJonathan
Type: New feature | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: GSoC | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/15787 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:12>
Django
-------------------------------------+-------------------------------------
| AllenJonathan
Type: New feature | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: GSoC | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"e64919ae54933ef4840f0e27e51d9fcfd55ecf4b" e64919ae]:
{{{
#!CommitTicketReference repository=""
revision="e64919ae54933ef4840f0e27e51d9fcfd55ecf4b"
Refs #29799 -- Added more tests for registering lookups.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:13>
Django
-------------------------------------+-------------------------------------
| AllenJonathan
Type: New feature | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:14>
Django
-------------------------------------+-------------------------------------
| AllenJonathan
Component: Database layer | Version: dev
(models, ORM) |
Keywords: GSoC | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
See
https://github.com/django/django/commit/cd1afd553f9c175ebccfc0f50e72b43b9604bd97.
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:15>
Django
-------------------------------------+-------------------------------------
| AllenJonathan
Type: New feature | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: GSoC | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by GitHub <noreply@…>):
In [changeset:"3afdc9e9b47d5bdd1bd653633b4cb2357478ade5" 3afdc9e9]:
{{{
#!CommitTicketReference repository=""
revision="3afdc9e9b47d5bdd1bd653633b4cb2357478ade5"
Refs #29799 -- Added field instance lookups to suggestions in FieldErrors.
Bug in cd1afd553f9c175ebccfc0f50e72b43b9604bd97.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:16>
Django
-------------------------------------+-------------------------------------
| AllenJonathan
Type: New feature | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: GSoC | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"be6a309b1d749c47821dfdc5add6576f2c61cda0" be6a309]:
{{{
#!CommitTicketReference repository=""
revision="be6a309b1d749c47821dfdc5add6576f2c61cda0"
[4.2.x] Refs #29799 -- Added field instance lookups to suggestions in
FieldErrors.
Bug in cd1afd553f9c175ebccfc0f50e72b43b9604bd97.
Backport of 3afdc9e9b47d5bdd1bd653633b4cb2357478ade5 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29799#comment:17>