Opened 11 years ago

Last modified 10 years ago

#20057 new Bug

Reverse related manager should be a manager INSTANCE, not CLASS

Reported by: Jonas H. Owned by: nobody
Component: Database layer (models, ORM) Version: 1.5
Severity: Normal Keywords:
Cc: odin.omdal@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Please see https://github.com/carljm/django-model-utils/issues/31 for a problem description -- "Solution 1" is what this bug is filed for.

What I currently think of is actually pretty simple:

For the descriptor runtime related manager class hackery (this stuff here https://github.com/django/django/blob/d744c550d51955fd623897884446b7f34318e94d/django/db/models/fields/related.py#L484), don't subclass the original manager and then call @super(...)@ in the methods. Instead, make it a totally new class that calls instance methods.

We would probably have to extend the wrappery to replace inheritance. Or do some other type-hackery. (It can probably not get any uglier than it already is ;-)

Change History (4)

comment:1 by Jonas H., 11 years ago

Summary: Reverse related manager should be on manager INSTANCE, not CLASSReverse related manager should be a manager INSTANCE, not CLASS

comment:2 by Carl Meyer, 11 years ago

Triage Stage: UnreviewedAccepted

I haven't looked into this enough yet to really evaluate the feasibility of turning related managers into instance wrappers rather than subclasses, but I know the current system is a major hassle for custom managers that you want used for related-object queries, because it effectively means you can't do any important configuration of the manager via instantiation (since the related manager will be a new instance of a subclass). This is wasteful: since you're instantiating the manager when you assign it as a model class attribute, you ought to be able to usefully pass it arguments there. And it leads to nasty dynamic-subclass-creation hacks to achieve things that ought to be doable via simple instantiation arguments. (See https://github.com/carljm/django-model-utils/blob/master/model_utils/managers.py#L172). So I'm accepting this on principle: if we can do it, it would be an improvement.

comment:3 by Anssi Kääriäinen, 11 years ago

FWIW I am very much +1 for changing the way the related managers are handled. As said above the code can't get any uglier than it already is...

comment:4 by Odin Hørthe Omdal, 10 years ago

Cc: odin.omdal@… added
Note: See TracTickets for help on using tickets.
Back to Top