[Django] #22111: Signal can throw ValueError in debug mode
Django
------------------------------+--------------------
Reporter: ilya_pirogov | Owner: nobody
Type: Bug | Status: new
Component: Python 3 | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------
Examples:
{{{
// accepts keyword-only arguments
@receiver(signals.post_save, sender=MyModel)
def my_handler(*, sender, instance, **kwargs):
pass
// contains annotations
@receiver(signals.post_save, sender=MyModel)
def my_handler(sender, instance: MyModel, **kwargs):
pass
}}}
In both cases the Signal can throw an error:
ValueError: Function has keyword-only arguments or annotations, use
getfullargspec() API which can support them
Pull request: https://github.com/django/django/pull/2336
--
Ticket URL: <https://code.djangoproject.com/ticket/22111>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: ilya_pirogov | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by bmispelon):
* needs_docs: => 0
* needs_better_patch: => 1
* version: 1.6 => master
* needs_tests: => 1
* stage: Unreviewed => Accepted
Comment:
Hi,
I can reproduce the issue indeed.
The approach taken in the pull request looks sane but needs some tests
before it can be merged.
And while we're there, I think there's a few other places in Django where
`getargspec` is used. Should those instances be fixed too?
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/22111#comment:1>
Django
Reporter: ilya_pirogov | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by ilya_pirogov):
Hello.
With tests there was a problem that syntax of annotations and keywords-
only arguments is incorrect for Python 2.
I created tests where handlers for both Pythons are imported from
different modules:
{{{
@skipIf(six.PY2, 'for Python 3 only')
class GetargspecPy3TestCase(SimpleTestCase):
def test_getargspec(self):
from .py3_handlers import (handler_with_kwargs_only,
handler_with_annotations)
# ...
@skipIf(six.PY3, 'for Python 2 only')
class GetargspecPy2TestCase(SimpleTestCase):
def test_getargspec(self):
from .py2_handlers import handler_simple
# ...
}}}
As far as this decision is correct in Django?
Yes, I think other cases are also affect this problem.
I wrote compatible implementation of `getargspec` and placed it in
`django.utils.inspect` module. Then I replaced `import inspect` with this
module.
This is an acceptable solution?
I committed all this on github.
--
Ticket URL: <https://code.djangoproject.com/ticket/22111#comment:2>
Django
Reporter: ilya_pirogov | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timo):
I don't think there's a need to alias all of `inspect` under
`django.utils.inspect`. Maybe the fix is too specific to Django, but I
wonder if it wouldn't be better to try to include this in a Python 2/3
compatibility library like [http://pythonhosted.org/six/ six].
--
Ticket URL: <https://code.djangoproject.com/ticket/22111#comment:3>
Django
Reporter: ilya_pirogov | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
Yes, we avoid maintaining patched versions of stdlib features at all
costs.
--
Ticket URL: <https://code.djangoproject.com/ticket/22111#comment:4>
Django
Reporter: ilya_pirogov | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* component: Python 3 => Core (Other)
--
Ticket URL: <https://code.djangoproject.com/ticket/22111#comment:5>
Django
Reporter: ilya_pirogov | Owner: nobody
Type: Bug | Status: closed
Component: Core (Other) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by vinnyrose):
* status: new => closed
* resolution: => fixed
Comment:
Fixed in Django 1.9+ with #24979
--
Ticket URL: <https://code.djangoproject.com/ticket/22111#comment:6>