[Django] #25897: Managers defined on non-abstract base classes are in fact inherited by child classes.
Django
child classes.
----------------------------------------------+----------------------------
Reporter: poleha | Owner: poleha
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: master
Severity: Normal | Keywords: manager,
| inheritance
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+----------------------------
Although documentation says it shouldn't:
https://docs.djangoproject.com/en/dev/topics/db/managers/#custom-managers-
and-model-inheritance
--
Ticket URL: <https://code.djangoproject.com/ticket/25897>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
child classes.
Reporter: poleha | Owner: poleha
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: manager, | Triage Stage: Accepted
inheritance |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by sir-sigurd):
* needs_better_patch: => 0
* version: master => 1.8
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
steps to reproduce:
{{{
class TestManager(models.Manager):
pass
class Test(models.Model):
objects = TestManager()
other_objects = TestManager()
class TestInherited(Test):
pass
}}}
{{{
In [2]: TestInherited.objects
Out[2]: <django.db.models.manager.Manager at 0x7fd8e8ae2550>
In [3]: TestInherited.other_objects
Out[3]: <geotest.models.TestManager at 0x7fd8e8a74390>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25897#comment:1>
Django
child classes.
Reporter: poleha | Owner: poleha
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: manager, | Triage Stage: Accepted
inheritance |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by poleha):
Pull request:
https://github.com/django/django/pull/5797
--
Ticket URL: <https://code.djangoproject.com/ticket/25897#comment:2>
Django
child classes.
Reporter: poleha | Owner: poleha
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: manager, | Triage Stage: Accepted
inheritance |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by sir-sigurd):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25897#comment:3>
Django
child classes.
Reporter: poleha | Owner: poleha
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: manager, | Triage Stage: Accepted
inheritance |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Is there any indication this is a regression such that the fix should be
backported or should we instead correct older versions of the
documentation?
--
Ticket URL: <https://code.djangoproject.com/ticket/25897#comment:4>
Django
child classes.
Reporter: poleha | Owner: poleha
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: manager, | Triage Stage: Accepted
inheritance |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by poleha):
Replying to [comment:4 timgraham]:
> Is there any indication this is a regression such that the fix should be
backported or should we instead correct older versions of the
documentation?
I found mentioning this:
>Managers defined on non-abstract base classes are not inherited by child
classes
in documentation since version 1.4:
https://docs.djangoproject.com/en/1.4/topics/db/managers/#custom-managers-
and-model-inheritance
And this bug exists in all versions since 1.4.
I think that fixing this bug in current version without fixing the
documentation would be enough. But I am ready to provide path for all
versions since 1.4. And I don't think that fixing old versions of
documentation would be correct in this case.
--
Ticket URL: <https://code.djangoproject.com/ticket/25897#comment:5>
Django
child classes.
Reporter: poleha | Owner: poleha
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: manager, | Triage Stage: Accepted
inheritance |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Looking at the original commit where the documentation was added,
f31425e8e23621fd4329c7377c9e220f526a1c49, it looks like named managers
were always inherited. What does seem to work is that the default manager
is never inherited.
The test case in the commit looks like this:
{{{
class Parent(models.Model):
manager = OnlyFred()
# Will not inherit default manager from parent.
class Child7(Parent):
pass
>>> Parent._default_manager.all()
[<Parent: fred>]
>>> Child7._default_manager.order_by('name')
[<Child7: barney>, <Child7: fred>]
>>> Child7.manager.all()
[<Parent: fred>]
>>> Child7._default_manager.order_by('name')
[<Child7: barney>, <Child7: fred>]
}}}
I wonder if the behavior should actually be changed (which will cause
backwards incompatibility for anyone relying on it) or if the
documentation should be fixed? Maybe we should ask on the
DevelopersMailingList.
--
Ticket URL: <https://code.djangoproject.com/ticket/25897#comment:6>
Django
child classes.
Reporter: poleha | Owner: poleha
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: manager, | Triage Stage: Accepted
inheritance |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
There hasn't been any immediate feedback
[https://groups.google.com/d/topic/django-
developers/QRvSCTM4WDo/discussion on the mailing list] except for me
suggesting to put this through the deprecation cycle. The patch is updated
for that but needs documentation as noted on the pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/25897#comment:7>
Django
child classes.
Reporter: poleha | Owner: poleha
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: manager, | Triage Stage: Accepted
inheritance |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by ar45):
* cc: aronp@… (added)
Comment:
Related to /Fixes #20932
--
Ticket URL: <https://code.djangoproject.com/ticket/25897#comment:8>
Django
child classes.
Reporter: poleha | Owner: poleha
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: manager, | Triage Stage: Accepted
inheritance |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by loic):
* cc: loic (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25897#comment:9>
Django
child classes.
Reporter: poleha | Owner: poleha
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by timgraham):
* keywords: manager, inheritance => manager-inheritance
--
Ticket URL: <https://code.djangoproject.com/ticket/25897#comment:10>
Django
child classes.
Reporter: poleha | Owner: poleha
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: manager-inheritance | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by loic):
[https://github.com/django/django/pull/6175#issuecomment-219451336 PR6175]
revamps manager inheritance and inheriting managers from non-abstract base
classes is the new expected behavior.
--
Ticket URL: <https://code.djangoproject.com/ticket/25897#comment:11>
Django
child classes.
Reporter: poleha | Owner: poleha
Type: Bug | Status: closed
Component: Database layer | Version: 1.8
(models, ORM) |
Keywords: manager-inheritance | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by Loïc Bistuer <loic.bistuer@…>):
In [changeset:"ed0ff913c648b16c4471fc9a9441d1ee48cb5420" ed0ff91]:
{{{
#!CommitTicketReference repository=""
revision="ed0ff913c648b16c4471fc9a9441d1ee48cb5420"
Fixed #10506, #13793, #14891, #25201 -- Introduced new APIs to specify
models' default and base managers.
This deprecates use_for_related_fields.
Old API:
class CustomManager(models.Model):
use_for_related_fields = True
class Model(models.Model):
custom_manager = CustomManager()
New API:
class Model(models.Model):
custom_manager = CustomManager()
class Meta:
base_manager_name = 'custom_manager'
Refs #20932, #25897.
Thanks Carl Meyer for the guidance throughout this work.
Thanks Tim Graham for writing the docs.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25897#comment:13>
Django
child classes.
Reporter: poleha | Owner: poleha
Type: Bug | Status: closed
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: manager-inheritance | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Loïc Bistuer <loic.bistuer@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"3a47d42fa33012b2156bf04058d933df6b3082d2" 3a47d42]:
{{{
#!CommitTicketReference repository=""
revision="3a47d42fa33012b2156bf04058d933df6b3082d2"
Fixed #20932, #25897 -- Streamlined manager inheritance.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25897#comment:12>