On Fri, Jun 1, 2012 at 8:53 AM, Jeremy Dunck <[email protected]> wrote:
> It feels to me that each place that ._default_manager is mentioned
> here is a misfeature:
> https://github.com/django/django/blob/2cd516002d43cdc09741618f0a0db047ee6d78fd/django/db/models/fields/related.py
>
> As an example, we (Votizen) currently have some default managers which
> show subsets of all objects (published, for example).
>
> class Candidate(...):
> races = ManyToMany(Race, related_name='candidates')
>
> objects = CandidateWithPublishedRacesManager()
> objects_all = Manager()
>
> class Race(...):
> objects = PublishedRaceManager()
> objects_all = Manager()
>
> The intention here is that we show the blessed subset by default
> (mostly for the site), show more for admin, shell, scripts, etc.
>
> The trouble is this asymmetry:
>
> candidate_with_unpublished_races = Candidate.objects_all.all()[0]
>
> candidate_with_unpublished_races.races.all() !=
> Race.objects.filter(candidates=candidate_with_unpublished_races)
>
> This affects admin (because while we can control
> CandidateAdmin.queryset, we can't control Candidate.races). But it
> also comes as a surprise fairly often in general. Note that no error
> is raised, you just process a subset of the data you meant to,
> generally.
>
> (We have considered instead adding manager methods for the site:
> Candidate.objects.for_site()..., but you see this has the same problem
> - assuming the default manager doesn't call that chained query
> method.)
>
>
> OK, for a solution, here's an API I think would remove the asymmetry:
>
>
> Candidate.context('site') would return a manager depending on the
> requested context.
> (or perhaps Candidate.objects.using(context='site')... or
> Candidate.objects.context('site'))
>
> # get an instance while assigning ._state.context == 'site', similar
> to ._state.db
> candidate_with_unpublished_races = Candidate.context('site')
>
> # the router grows a method to help decide what to do on a per-model basis.
> class DatabaseRouter(object):
> ...# as now except, for example:
> def manager_for_context(self, context, model_class):
> # returns a manager
> # e.g.
> return getattr(model_class, "objects_%s" % context)
> # could also by dynamically constructed.
>
> (It seems to me the router is the natural place to have
> project-specific customization of what happens with managers, though
> perhaps some community norms would be needed
>
> And then perhaps models can have defaults, as they do DBs:
>
> class FooModel(Model):
> ...
> class Meta:
> context = "site"
>
> class FooAdmin(Admin):
> model = FooModel
>
> class Meta:
> context = "admin"
>
> For backwards-compatibility, any model without a context specified in
> meta gets "default", and the default database manager always resolves
> to model_class._default_manager.
>
> For app composition, there is still value in the concept of a default
> manager, though I'm not entirely clear how this fits together.
>
> Perhaps Frob.objects would resolve to some manager based on the default
> context,
> unless .objects had been explicitly assigned to be a vanilla manager
> (much as a queryset resolves the db via the router unless .using(db=)
> has been used).
>
> Note, I'm not entirely happy with this, but it gives something to start from.
>
> Thoughts?
Is this really something that's going to be able to be managed at the
routing level? It seems to me that the decision about exactly which
set of objects you want returned needs much finer control that a
router method will allow.
It might not happen in your case specifically, but I wouldn't think it
would be unusual to have a single view in which both "full" list and
the "filtered" list are retrieved. If you're working with the managers
directly, that's easy:
selected_canddiates = Candidate.objects.all()
all_candidates = Candidate.objects_all.all()
but if you want to go through the relation for objects related to a
specific race, it's not:
selected_canddiates = race.candidates.all()
all_candidates = race.candidates.all() # this won't give the right results...
If I'm understanding your proposal correctly, I'd need to either tweak
the _state of race between the two calls to switch the context, or
write a router that was able to differentiate between the two uses
based on some other contextual cue (I'm not sure what could be
provided to give that differentiation, though).
The context switching might be possible with some utility methods or a
Python context managers, but it still seems like the wrong way to
affect control -- modifying an object's state so that it's default
behaviour changes, rather than explicitly requesting the behaviour
your want.
Here's a counter-proposal:
mycandidate.races is a descriptor. That descriptor implements
__get__() to return a RelatedObjectsManager that does the appropriate
filtering. The related objects manager is a standard manager that
allows calls to all(), filter() etc. Why not also implement __call__,
and have __call__() return a new RelatedObjectsManager, bound to a
different manager (as named in the arguments to call)?
So, using your example:
* race.candidates.all() would use _default_manager
* race.candidates('objects_all').all() would use the objects_all manager
* race.candidates('objects').all() would also use _default_manager
(but explicitly)
With some slightly neater manager naming, it becomes even easier to
read. Consider the following example:
class Person(…):
people = Manager()
men = MaleManager()
women = FemaleManager()
class Event(…):
attendees = ManyToMany(Person, related_name='events')
objects = Manager()
confirmed = ConfirmedManager()
then:
all people: Person.people.all()
all women: Person.women.all()
all events: Event.objects.all()
all confirmed events: Event.confirmed.all()
all events being attended by the Person frank: frank.events.all()
all confirmed events being attended by the Person frank:
frank.events('confirmed').all()
all people attending the brogrammer event: brogrammer.attendees.all()
all men attending the brogrammer event: brogrammer.attendees('men').all()
For my money, that's an explicit API that makes sense when reading the
single query line, and doesn't require the interpolation of extra
state/context from a router or some other context manager. It's also
backwards compatible, because there won't be any existing code
invoking __call__() on a manager.
Russ %-)
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
