[Django] #27350: Attaching signals to abstract models don't work as it used to be

5 views
Skip to first unread message

Django

unread,
Oct 14, 2016, 11:19:35 AM10/14/16
to django-...@googlegroups.com
#27350: Attaching signals to abstract models don't work as it used to be
----------------------------------------------+--------------------
Reporter: thoas | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
Hi,

I'm currently porting an application from Django 1.9.x to 1.10.x, we are
relying on managers to attach signals to allow developers overriding them
in our generic applications, this does not work anymore.

A complete example:


{{{
from django.db import models
from django.db.models import signals


class Poll(models.Model):
question = models.CharField(max_length=255)
answers_count = models.PositiveIntegerField(default=0)


class AnswerManager(models.Manager):
def contribute_to_class(self, cls, name):
signals.post_save.connect(self.post_save, sender=cls)

return super(AnswerManager, self).contribute_to_class(cls, name)

def post_save(self, instance, **kwargs):
poll = instance.poll
poll.answers_count = poll.answers.count()
poll.save(update_fields=('answers_count', ))


class AbstractAnswer(models.Model):
text = models.CharField(max_length=255)
poll = models.ForeignKey(Poll, related_name='answers')

objects = AnswerManager()

class Meta:
abstract = True


class Answer(AbstractAnswer):
class Meta:
abstract = False


class ModelsTests(TestCase):
def test_simple_compute(self):
poll = Poll.objects.create(question='Weird?')

Answer.objects.create(poll=poll, text='Yes')

# refresh
poll = Poll.objects.get(pk=poll.pk)

# still 0 on Django 1.10 and 1 on Django 1.9
assert poll.answers_count == 1
}}}


The complete application is available [[https://github.com/thoas/django-
signals-wtf|here]] and travis is available [[https://travis-ci.org/thoas
/django-signals-wtf/builds/167669153|here]] to test again both 1.10 and
1.9.

Is this some kind of undocumented new behavior or a bug?

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

Django

unread,
Oct 14, 2016, 11:25:16 AM10/14/16
to django-...@googlegroups.com
#27350: Attaching signals to abstract models don't work as it used to be
-------------------------------------+-------------------------------------
Reporter: Florent Messa | 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):

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0


Comment:

Hi, you should
[https://docs.djangoproject.com/en/dev/internals/contributing/triaging-
tickets/#bisecting-a-regression bisect] to find the commit where the
behavior changed. If I had to guess it might be
ed0ff913c648b16c4471fc9a9441d1ee48cb5420 or
3a47d42fa33012b2156bf04058d933df6b3082d2.

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

Django

unread,
Oct 14, 2016, 11:55:27 AM10/14/16
to django-...@googlegroups.com
#27350: Attaching signals to abstract models don't work as it used to be
-------------------------------------+-------------------------------------
Reporter: Florent Messa | 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 Florent Messa):

Replying to [comment:1 Tim Graham]:


> Hi, you should
[https://docs.djangoproject.com/en/dev/internals/contributing/triaging-
tickets/#bisecting-a-regression bisect] to find the commit where the
behavior changed. If I had to guess it might be
ed0ff913c648b16c4471fc9a9441d1ee48cb5420 or
3a47d42fa33012b2156bf04058d933df6b3082d2.

It was introduced by 3a47d42fa33012b2156bf04058d933df6b3082d2

Build is available on [[https://travis-ci.org/thoas/django-signals-
wtf/builds/167683086|travis]], **110a** is the parent commit of
3a47d42fa33012b2156bf04058d933df6b3082d2 and **110b** is located at
3a47d42fa33012b2156bf04058d933df6b3082d2

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

Django

unread,
Oct 14, 2016, 12:02:16 PM10/14/16
to django-...@googlegroups.com
#27350: Attaching signals to abstract models don't work as it used to be
-------------------------------------+-------------------------------------
Reporter: Florent Messa | 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:

Looking at your code, I don't think we broke any public APIs, so I'd say
it's up to you to investigate and say why it's a bug and to propose a fix.
Maybe Loic can advise without too much effort.

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

Django

unread,
Oct 14, 2016, 12:18:44 PM10/14/16
to django-...@googlegroups.com
#27350: Attaching signals to abstract models don't work as it used to be
-------------------------------------+-------------------------------------
Reporter: Florent Messa | 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 Florent Messa):

Replying to [comment:3 Tim Graham]:


> Looking at your code, I don't think we broke any public APIs, so I'd say
it's up to you to investigate and say why it's a bug and to propose a fix.
Maybe Loic can advise without too much effort.

The **contribute_to_class** is called twice in 1.9.x on the abstract level
and child.

That's not the case on 1.10.x your child model needs to declare explicitly
the manager even if it's inherited.

I can update my code on my side on every child to redeclare the manager,
as you have said it's not a public API as it's not documented. I can
investigate more on my side and provide a fix for that but will you
consider it?

Btw, thank you for you reactivity ;)

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

Django

unread,
Oct 14, 2016, 12:40:58 PM10/14/16
to django-...@googlegroups.com
#27350: Attaching signals to abstract models don't work as it used to be
-------------------------------------+-------------------------------------
Reporter: Florent Messa | 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 Tim Graham):

I didn't follow the manager changes closely enough to say what the
expected behavior is.

--
Ticket URL: <https://code.djangoproject.com/ticket/27350#comment:5>

Django

unread,
Oct 22, 2016, 7:04:05 PM10/22/16
to django-...@googlegroups.com
#27350: Attaching signals to abstract models don't work as it used to be
-------------------------------------+-------------------------------------
Reporter: Florent Messa | 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


--
Ticket URL: <https://code.djangoproject.com/ticket/27350#comment:6>

Reply all
Reply to author
Forward
0 new messages