Signal can throw ValueError in debug mode

[Ilya Pirogov]

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

[Baptiste Mispelon]

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.

[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.

[Tim Graham]

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 ​six.

[Aymeric Augustin]

Yes, we avoid maintaining patched versions of stdlib features at all costs.

[vinnyrose]

Fixed in Django 1.9+ with #24979