Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#19384 closed Bug (fixed)

Managers can no longer be used on abstract models

Reported by: mhsparks Owned by: nobody
Component: Database layer (models, ORM) Version: master
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 Changed 3 years ago by russellm

  • Component changed from Uncategorized to Database layer (models, ORM)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Design decision needed
  • Type changed from Uncategorized to Bug

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 Changed 3 years ago by mhsparks

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 Changed 3 years ago by danfairs

  • Cc dan.fairs@… added

comment:4 Changed 3 years ago by ptone

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 Changed 3 years ago by aaugustin

I agree with ptone.

comment:6 Changed 3 years ago by russellm

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

comment:7 Changed 3 years ago by Russell Keith-Magee <russell@…>

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

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 Changed 3 years ago by Russell Keith-Magee <russell@…>

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