Code

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#9676 closed (fixed)

Explicit default managers

Reported by: mrts Owned by: mtredinnick
Component: Database layer (models, ORM) Version: 1.0
Severity: Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Currently, "the first manager declared is the default manager".

However, given the following hierarchy:

class PublishedObjectManager(models.Manager):
    def get_query_set(self):
        return super(PublishedObjectManager, self).get_query_set()\
                .filter(is_published=True)

class PublicationBase(models.Model):
    ...
    objects = models.Manager()
    published_objects = PublishedObjectManager()

    class Meta:
        abstract = True

class ToplevelPageManager(PublishedObjectManager):
    def get_query_set(self):
        return super(ToplevelPageManager, self).get_query_set()\
                .filter(parent=None)

class PageBase(PublicationBase):
    ...
    toplevel_pages = ToplevelPageManager()

    class Meta:
        abstract = True

all classes inheriting from PageBase get toplevel_pages as their
default manager.
To make objects default, repetition is necessary:

class PageBase(PublicationBase):
    ...
    objects = models.Manager()
    toplevel_pages = ToplevelPageManager()

    class Meta:
        abstract = True

only to specify the default manager. A way to explicitly say
"this is the default manager that should be honoured throughout the
inheritance hierarchy" would be appropriate. Also, the _ in front of
_default_manager implies that it is an implementation detail, neither is
accessing default managers documented. But all reusable app
writers should really use model._default_manager instead of
model.objects for any foreign models they must handle.

Generally, there should be an explicit, documented way to set and get
the default manager.

Attachments (0)

Change History (7)

comment:1 Changed 6 years ago by anonymous

  • Needs documentation set
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 6 years ago by mrts

Perhaps it is better to just make objects the (overrideable) default manager as suggested in http://groups.google.com/group/django-developers/browse_thread/thread/480fe23f12e55cf7 . I.e. the concept needs a little thought, care and love.

comment:3 Changed 5 years ago by jacob

  • Triage Stage changed from Unreviewed to Design decision needed

comment:4 Changed 5 years ago by mtredinnick

I'm not going to worry about whether or not to make _default_manager public here, since that isn't a well defined problem at the moment, so a "fix" isn't going to clearly correct or incorrect (working out whether access to that parameter is part of the problem). Normally, if you're referring to some other model you either know the model you're referring to, so can use the right manager, or it's a relation, in which case the manager choice is automatic. It's only real edge-cases that need to understand the concept of a default manager and won't know it's name (such as the admin) and they are a corner case in the application space. I've moved that to #10499 and leaving this ticket as being about interleaving new default managers.

The other problem is already possible to solve using class inheritance (mixins, since it's mixing in a new default manager to the default hierarchy). No new feature is required there. I'm about to commit some documentation to explain that pattern explicitly in the documentation.

comment:5 Changed 5 years ago by mtredinnick

  • Owner changed from nobody to mtredinnick
  • Status changed from new to assigned

Run out of time today. I'll commit this change tomorrow; I've got it done and ready to go.

comment:6 Changed 5 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [10058]) Documented patterns for adding extra managers to model subclasses.

This seems to have been a source of confusion, so now we have some explicit
examples. Fixed #9676.

comment:7 Changed 5 years ago by mtredinnick

(In [10061]) [1.0.X] Documented patterns for adding extra managers to model subclasses.

This seems to have been a source of confusion, so now we have some explicit
examples. Fixed #9676.

Backport of r10058 from trunk.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.