Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#19384 closed Bug (fixed)

Managers can no longer be used on abstract models

Reported by: Mark Hughes Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Release blocker Keywords:
Cc: dan.fairs@… Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Given the (contrived) models.py

from django.db import models

class ChoiceManager(models.Manager):

    def get_first_choice(self):
        return Choice1

class Choice(models.Model):
    choice = models.CharField(max_length=200)
    votes = models.IntegerField()

    choices = ChoiceManager()

    class Meta:
        abstract = True

class Choice1(Choice):
    pass

class Choice2(Choice):
    pass


I get the following error when trying to access models.Choice.choices.get_first_choice() using 1.5 (master).

Traceback (most recent call last):
  File "/home/mark/venvs/django-14/abstract_managers/demo/tests.py", line 20, in test_abstract_manager
    models.Choice.choices.get_first_choice()
  File "/home/mark/venvs/django-15/src/django/django/db/models/manager.py", line 244, in __get__
    self.model._meta.object_name,
AttributeError: Manager isn't available; Choice is abstract

The code runs fine under 1.4.

It appears that this behaviour was changed with this commit:

https://github.com/django/django/commit/cc337a74f1808b216fff96f1695d8b066d2636f6

Relating to this ticket:

https://code.djangoproject.com/ticket/19069

Change History (8)

comment:1 by Russell Keith-Magee, 11 years ago

Component: UncategorizedDatabase layer (models, ORM)
Severity: NormalRelease blocker
Triage Stage: UnreviewedDesign decision needed
Type: UncategorizedBug

I'm marking this as a release blocker, because it's clearly a change in behavior between versions.

However, I'm not completely convinced (yet) that the problem use case is a use case we intended to support. Calling a manager on the abstract isn't something that *should* be possible, AFAICT, because there's nothing for the abstract class to manage. You can't issue queries on the abstract model, for example. The use case here is to use the manager as a mechanism for getting at a specific class object -- but I'm not sure I see what that shouldn't be a normal class method, rather than a manager method.

I appreciate that you've gone to the lengths of producing a simplified example here -- that definitely helps reproduce the problem. Can you elaborate some more on why you're using this approach in practice? I don't need to see code -- just an explanation of what you're putting on the abstract manager that needs to be accessed on the abstract base class.

comment:2 by Mark Hughes, 11 years ago

Thanks for taking the time to look at this. We're using the abstract class manager to access partitioned versions of the model. Some context to try and explain further:

Our app runs many similar fantasy sports and tipping games. In these users make a series of choices for a round and compete with others over a number of phases. We have implemented a partition manager that allows very large tables such as tips and league standings to be partitioned over separate tables based on round or phase rather than simply have round or phase as a foreign key.

During start-up (model generation phase) N models will be created based on how the abstract base model has been defined. For example if a game has 40 Rounds and the Tip model has been configured to partition by Round then Tip1 to Tip40 models will be created and the manager is then used to access these partitions, similarly if a game has 10 Phases LeagueStanding1 to LeagueStanding10 models will be created and the LeagueStanding partitions manager used to access them.

If we weren't using a model manager then we'd probably need to use a mixin class instead to implement similar behaviour in all models that need it.

comment:3 by Dan Fairs, 11 years ago

Cc: dan.fairs@… added

comment:4 by Preston Holmes, 11 years ago

Personally I think we need to document this as a backwards incompatible cleanup.

I don't see what advantage manager methods have for the presented use case over a standard python method on the abstract base class that returns or calls the desired manager on the concrete subclass.

comment:5 by Aymeric Augustin, 11 years ago

I agree with ptone.

comment:6 by Russell Keith-Magee, 11 years ago

I agree with ptone and aaugustin. Documentation patch incoming shortly.

comment:7 by Russell Keith-Magee <russell@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 1b646e656e46909af447056b4f98b9744aae4978:

Fixed #19384 -- Documented the behavior of custom managers on abstract models.

This documents the behavior introduced by cc337a74, which is BACKWARDS
INCOMPATIBLE for any attempt to invoke a method on a manager using the
abstract class as the calling class (e.g., AbstractBase.objects.do_something())

Thanks to mhsparks for the report.

comment:8 by Russell Keith-Magee <russell@…>, 11 years ago

In 9cf566233bb5ad16c36329645fc6090577fb42f6:

[1.5.X] Fixed #19384 -- Documented the behavior of custom managers on abstract models.

This documents the behavior introduced by cc337a74, which is BACKWARDS
INCOMPATIBLE for any attempt to invoke a method on a manager using the
abstract class as the calling class (e.g., AbstractBase.objects.do_something())

Thanks to mhsparks for the report.

Backport of 1b646e656e46909af447056b4f98b9744aae4978 from master.

Note: See TracTickets for help on using tickets.
Back to Top