[Django] #25279: Make prefetch_related_objects a public API
Django
----------------------------------------------+--------------------
Reporter: adamchainz | Owner: nobody
Type: New feature | Status: new
Component: Database layer (models, ORM) | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
`prefetch_related` is great, but it would be nice to be able to reuse its
capabilities with lists of model instances already retrieved by other code
(or from cache, or newly constructed, etc.). It seems this is possible by
using `django.db.models.query.prefetch_related_objects` which is the
function implementing all the prefetch capability - I've just seen it used
fine, however it is not part of Django's public API.
As far as I can tell, just doing an `import` in
`django.db.models.__init__` and adding documentation should be enough to
make it public, since `prefetch_related_objects` contains all the
functionality that `prefetch_related` exposes.
--
Ticket URL: <https://code.djangoproject.com/ticket/25279>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: adamchainz | Owner: nobody
Type: New feature | Status: new
(models, ORM) |
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 timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Seems related to #24975 which proposes to add the ability to call
`prefetch_related()` on a model instance. I wonder if we could find a
common solution to address both of these tickets.
If we make `prefetch_related_objects()` public, I think some tests would
also be needed to ensure that it isn't refactored so that it's signature
changes.
--
Ticket URL: <https://code.djangoproject.com/ticket/25279#comment:1>
Django
Reporter: adamchainz | Owner: nobody
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by adamchainz):
* has_patch: 0 => 1
Comment:
Oh #24975 is nice. It doesn't seem that hard to do - on Model would it not
just require...
{{{
def prefetch_related(self, related_lookups):
prefetch_related_objects([self], related_lookups)
}}}
Basic PR at [https://github.com/django/django/pull/5138], with a
documentation sketch. Agree that more tests would be required.
--
Ticket URL: <https://code.djangoproject.com/ticket/25279#comment:2>
Django
Reporter: adamchainz | Owner: nobody
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_tests: 0 => 1
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/25279#comment:3>
Django
Reporter: adamchainz | Owner: nobody
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by adamchainz):
I've added some tests for the basics (one-to-one/many-to-many traversal
plus `Prefetch` objects) but don't want to go too far basically
duplicating the tests that already exist for `prefetch_related`. The aim
is just to check it accepts the same arguments and thought is put in to
changing it in the future.
I've also changed the function signature to
`prefetch_related_objects(list_of_objects, *lookups)` for the simpler
invocation `prefetch_related_objects(objs, 'lookup1', 'lookup2__and3')`,
as opposed to `prefetch_related_objects(objs, ['lookup1',
'lookup2__and3'])`. This is to defend against the possible bug from
`prefetch_related_objects(objs, 'lookup1')` which would iterate the
characters in `lookup1` leading to errors about attribute `l` etc. not
existing.
I'm guessing that although some existing projects may already be using
`prefetch_related_objects`, it's not Django's policy to worry about
changing the signature of a (previously) private function? Or should I add
type-checking like `if len(args) == 1 and isinstance(args[0], (tuple,
list))` to allow the old style?
--
Ticket URL: <https://code.djangoproject.com/ticket/25279#comment:4>
Django
Reporter: adamchainz | Owner: nobody
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/25279#comment:5>
Django
Reporter: adamchainz | Owner: nobody
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Description changed by timgraham:
Old description:
> `prefetch_related` is great, but it would be nice to be able to reuse its
> capabilities with lists of model instances already retrieved by other
> code (or from cache, or newly constructed, etc.). It seems this is
> possible by using `django.db.models.query.prefetch_related_objects` which
> is the function implementing all the prefetch capability - I've just seen
> it used fine, however it is not part of Django's public API.
>
> As far as I can tell, just doing an `import` in
> `django.db.models.__init__` and adding documentation should be enough to
> make it public, since `prefetch_related_objects` contains all the
> functionality that `prefetch_related` exposes.
New description:
`prefetch_related` is great, but it would be nice to be able to reuse its
capabilities with lists of model instances already retrieved by other code
(or from cache, or newly constructed, etc.). It seems this is possible by
using `django.db.models.query.prefetch_related_objects` which is the
function implementing all the prefetch capability - I've just seen it used
fine, however it is not part of Django's public API.
As far as I can tell, just doing an `import` in
`django.db.models.__init__` and adding documentation should be enough to
make it public, since `prefetch_related_objects` contains all the
functionality that `prefetch_related` exposes.
[https://groups.google.com/d/topic/django-
developers/xjxKnhEDl7s/discussion django-developers discussion]
--
--
Ticket URL: <https://code.djangoproject.com/ticket/25279#comment:6>
Django
Reporter: adamchainz | Owner: nobody
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* keywords: => 1.9
Comment:
Loic had some doubts about making `prefetch_related_objects()` a public
API. So, I'm awaiting his feedback here or on the mailing list before
proceeding with this ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/25279#comment:7>
Django
Reporter: adamchainz | Owner: nobody
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* keywords: 1.9 =>
Comment:
Loic's concerns are, "I have nothing against exposing that functionality
itself, I just wanted to review if it works as-is as an elegant and
reliable public API, right now it's a utility function it wasn't designed
with those concerns in mind."
Since it's mostly a documentation patch at this point, it doesn't seem
critical for 1.9.
--
Ticket URL: <https://code.djangoproject.com/ticket/25279#comment:8>
Django
Reporter: adamchainz | Owner: nobody
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
Comment:
Pending Loic's review.
--
Ticket URL: <https://code.djangoproject.com/ticket/25279#comment:9>
Django
Reporter: adamchainz | Owner: nobody
Type: New feature | Status: closed
Component: Database layer | Version: 1.8
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"ef33bc2d4d5e66b08cba2a318aa700ba1e28ba81" ef33bc2]:
{{{
#!CommitTicketReference repository=""
revision="ef33bc2d4d5e66b08cba2a318aa700ba1e28ba81"
Fixed #25279 -- Made prefetch_related_objects() public.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25279#comment:10>