[Django] #27664: contribute_to_class is called with wrong model class

18 views
Skip to first unread message

Django

unread,
Dec 29, 2016, 11:21:12 AM12/29/16
to django-...@googlegroups.com
#27664: contribute_to_class is called with wrong model class
-------------------------------------+-------------------------------------
Reporter: Johannes | Owner: nobody
Hoppe |
Type: Bug | Status: new
Component: Database | Version: 1.10
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Since 1.10 `Manager.contribute_to_class` gets called with the model on
which the manager is defined instead of the subclass.
Before that it used to be called for all sub models with the proper class.

You can easily reproduce that by running this in both version 1.9 and
1.10. `model` will be different.

{{{
class MyManager(models.Manager):

def contribute_to_class(self, model, name):
super().contribute_to_class(model, name)
print(model)

class AbstractModel(models.Model):
objects = MyManager()
class Meta:
abstract = True

class ContcreteModel(AbstractModel):
pass
}}}

This change has not been documented in the release notes and also seems
counter intuitive. Contribute to class should be called with the concrete
model.

--
Ticket URL: <https://code.djangoproject.com/ticket/27664>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Dec 29, 2016, 11:38:38 AM12/29/16
to django-...@googlegroups.com
#27664: contribute_to_class is called with wrong model class
-------------------------------------+-------------------------------------
Reporter: Johannes Hoppe | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

* cc: Loic Bistuer (added)


Comment:

This changed in 3a47d42fa33012b2156bf04058d933df6b3082d2 - Fixed #20932,
#25897 -- Streamlined manager inheritance.

I'm uncertain if the new behavior is intended or if it needs clarification
in the release notes, however, as far as I know `contribute_to_class()` is
a private API so backwards compatibility and documentation changes there
isn't a high priority.

--
Ticket URL: <https://code.djangoproject.com/ticket/27664#comment:1>

Django

unread,
Dec 29, 2016, 11:39:37 AM12/29/16
to django-...@googlegroups.com
#27664: Manager.contribute_to_class() is called with abstract model rather than
concrete model

-------------------------------------+-------------------------------------
Reporter: Johannes Hoppe | Owner: nobody
Type: Bug | Status: new

Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

--
Ticket URL: <https://code.djangoproject.com/ticket/27664#comment:2>

Django

unread,
Dec 29, 2016, 12:01:50 PM12/29/16
to django-...@googlegroups.com
#27664: Manager.contribute_to_class() is called with abstract model rather than
concrete model
-------------------------------------+-------------------------------------
Reporter: Johannes Hoppe | Owner: nobody
Type: Bug | Status: new

Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Johannes Hoppe):

Hi Tim,

I agree, it is not documented, but I guess it's not to uncommon for third
party libraries to make use of `contribute_to_class` be it on `Managers`
or `Fields`.

Regardless, the documentation says, that the manager will be inherited. So
I would presume the having the attribute on a super class is the same than
having the attribute on the sub class.

This is not the case.

I'm not sure if this is just a matter of documentation or if it should be
changed. My first guess would be to change it. It seems more intuitive to
me. But I really like the that the new Manager design is less complex. If
there isn't an easy solution. This shouldn't be changed.

--
Ticket URL: <https://code.djangoproject.com/ticket/27664#comment:3>

Django

unread,
Jan 4, 2017, 5:28:29 PM1/4/17
to django-...@googlegroups.com
#27664: Manager.contribute_to_class() is called with abstract model rather than
concrete model
-------------------------------------+-------------------------------------
Reporter: Johannes Hoppe | Owner: nobody
Type: Bug | Status: closed

Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

* status: new => closed
* resolution: => needsinfo


Comment:

Absent a patch or explanation of why the change is needed, I don't know
what to do here. Feel free to investigate and offer a patch. I have a
feeling that "fixing" this requires reintroducing the complexity that
Loic's patch removed.

--
Ticket URL: <https://code.djangoproject.com/ticket/27664#comment:4>

Reply all
Reply to author
Forward
0 new messages