[Django] #32594: Signal.disconnect() returns None when passing sender as string
Django
-------------------------------------+-------------------------------------
Reporter: Hugo | Owner: Hugo Cachitas
Cachitas |
Type: Bug | Status: assigned
Component: Database | Version: 4.0
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
According to the documentation on `Signal.disconnect(receiver=None,
sender=None, dispatch_uid=None)`
The method returns True if a receiver was disconnected and False if not.
I am having this issue where `disconnect()` returns `None` when I specify
the sender as a string model reference, i.e., passing
`sender="myapp.MyModel"` instead of `sender=MyModel`.
The sender may be lazily specified as a string as documented in
`django.db.models.signals.ModelSignal`.
To achieve this functionality, a `_lazy_method` is implemented where we
can find that the return value is not properly set.
{{{
#!python
if isinstance(sender, str):
apps = apps or Options.default_apps
apps.lazy_model_operation(partial_method, make_model_tuple(sender))
else:
return partial_method(sender)
}}}
I already have a failing test at
`tests/signals/tests.py:LazyModelRefTests` that could use your input.
{{{
#!python
@isolate_apps('signals', kwarg_name='apps')
def test_disconnect_return_value(self, apps):
"""
Signal.disconnect() return value should be consistent if we
use the Model or its string reference.
"""
class Created(models.Model):
pass
def receiver(**kwargs):
pass
sender = Created
signals.post_init.connect(receiver, sender=sender, apps=apps)
self.assertTrue(
signals.post_init.disconnect(receiver, sender=sender, apps=apps)
)
self.assertFalse(
signals.post_init.disconnect(receiver, sender=sender, apps=apps)
)
sender = 'signals.Created'
signals.post_init.connect(receiver, sender=sender, apps=apps)
self.assertTrue(
signals.post_init.disconnect(receiver, sender=sender, apps=apps)
)
self.assertFalse(
signals.post_init.disconnect(receiver, sender=sender, apps=apps)
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32594>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
| Cachitas
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Hugo Cachitas):
PR [https://github.com/django/django/pull/14187]
The solution involved touching the app registry.
Tests are passing, but I am not entirely sure of the implications the new
intermediary returned values may have.
--
Ticket URL: <https://code.djangoproject.com/ticket/32594#comment:1>
Django
-------------------------------------+-------------------------------------
| Cachitas
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32594#comment:2>
Django
-------------------------------------+-------------------------------------
| Cachitas
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* keywords: => signals
--
Ticket URL: <https://code.djangoproject.com/ticket/32594#comment:3>
Django
-------------------------------------+-------------------------------------
| Cachitas
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Alex Hill (added)
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/32594#comment:4>
Django
-------------------------------------+-------------------------------------
| Cachitas
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: signals | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"f83214a3e15ccadc9e2370dd157ed19bcdb07e2c" f83214a3]:
{{{
#!CommitTicketReference repository=""
revision="f83214a3e15ccadc9e2370dd157ed19bcdb07e2c"
Refs #32594 -- Added Signal.disconnect() test with a model class.
Co-authored-by: Mariusz Felisiak <felisiak...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32594#comment:5>
Django
Reporter: Hugo Cachitas | Owner: Hugo Cachitas
Type: Bug | Status: assigned
Component: Documentation | Version: 4.0
Severity: Normal | Resolution:
Keywords: signals | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_better_patch: 0 => 1
* component: Database layer (models, ORM) => Documentation
* needs_docs: 0 => 1
Comment:
I think we should document the current behavior.
--
Ticket URL: <https://code.djangoproject.com/ticket/32594#comment:6>
Django
-------------------------------------+-------------------------------------
| Cachitas
Type: Bug | Status: assigned
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
* needs_docs: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/32594#comment:7>
Django
-------------------------------------+-------------------------------------
| Cachitas
Component: Documentation | Version: 4.0
Severity: Normal | Resolution: fixed
Keywords: signals | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"8f6a7a0e9e7c5404af6520ae606927e32415eb00" 8f6a7a0e]:
{{{
#!CommitTicketReference repository=""
revision="8f6a7a0e9e7c5404af6520ae606927e32415eb00"
Fixed #32594 -- Doc'd and tested that Signal.disconnect() with lazy
references returns None.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32594#comment:8>