#26749: Different manager for _base_manager and related descriptors
-------------------------------------+-------------------------------------
Reporter: spectras | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: manager | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by spectras):
Before changing anything, I must be certain to fully understand.
What I meant by "internal" is the way the manager is used does not expose
it to user code in cases 1-3. In those use cases, the manager provides a
service to Django ORM, which could completely be implemented with no
manager interaction, user code would not notice.
→ On the other hand, in cases 4-5, Django simply picks the manager and
passes it to user code, which makes it possible for the manager to expose
additional features to user code.
So we have two very different use patterns. Therefore it makes sense to
separate them — `related_manager` and `base_manager`. Actually, I'd even
say that `related_manager` is closer to `default_manager` than
`base_manager` in purpose and use pattern.
----
Let's get back to `base_manager`. Actually, cases 1-3 are not the whole
story, as you pointed out. It also serves the purpose of providing a
unfiltered-access-to-model guarantee.
I believe that's a big red flag. Because those two purposes are different:
cases 1-3 are consumed by the ORM while the unfiltered guarantee is
consumed by user code.
Now, this unfiltered-guarantee is clearly defined and definitely a good
addition to the public side of the API. No problem with that. Now, though,
the purpose of customizing the inner workings of the ORM seems to be
inconsistently served:
* Overriding insertion:
- Either override `MyModel._do_insert`
- or just let it forward the call to the base_manager and override
`MyBaseManager._insert`
* Overriding updates:
- Override `Queryset.update` (for queryset-based updates)
- and either:
- override `MyModel._do_update`
- or let it forward the call to base_manager's queryset and
override `QuerySet._update`
* Overriding deletion:
- If object can be fast deleted, override base_manager's queryset
`_raw_delete` method.
- otherwise, no luck, you cannot override deletion, it is a hardcoded
`sql.DeleteQuery(model)`.
There is no object clearly responsible for updating the database.
Sometimes the queryset does it, sometimes the base_manager, sometimes the
deletion collector bypasses both. Sometimes the calls go through the
model, with a default implementation that simply forwards them and
sometimes they don't.
Also, though `base_manager_name` is becoming a public API, doing anything
useful with it requires overriding private, undocumented API methods.
----
So, the final breakdown, as I understand it at the moment, is as such:
* `default_manager`: makes it possible to expose additional features to
user code, in the general case.
* `related_manager`: makes it possible to expose additional features to
user code, on related fields.
* `base_manager`: must provide the unfiltered, untampered guarantee for
use in things such as '''dumpdata'''.
* unspecified API clinging to `base_manager` and living in undocumented,
private API methods of the manager or its queryset: can be used to
override low-level operations. That is, cases 1-3.
I would advocate splitting `base_manager` and the unspecified API, for
several reasons:
1. it would make the `base_manager`'s role and responsibilities clearly
bound and defined.
2. there is no reason for those methods to live on the manager/queryset
in the first place.
3. the fact they live on the manager(/manager's queryset) means every
manager has them, yet they are only ever used/usable on the base
manager(/manager's queryset).
4. the unspecified API is inconsistent and incomplete anyway, certainly
not mature enough to go public with the `base_manager`.
Namely, I'd completely remove `Manager._insert` and `QuerySet._update` and
let `Model._do_insert` and `Model._do_update` do the job.
[[br]]
I hope this didn't sound too bold, I'm trying my best to make my meaning
unambiguous, somewhat at the expense of political correctness.[[br]]
If you think this has some value, I can try to make a proof of concept
patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/26749#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.83b484b443ea92154ca583a381a674e9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
