kumar303 / hawkrest

Hawk HTTP Authorization for Django Rest Framework
BSD 3-Clause "New" or "Revised" License
19 stars 8 forks source link

Middleware Not Compaitible with Django 1.11.5 final #38

Closed ydaniels closed 5 years ago

ydaniels commented 6 years ago

putting
'hawkrest.middleware.HawkResponseMiddleware' in middleware classes causes this error File "/usr/local/lib/python2.7/dist-packages/django/utils/autoreload.py", line 228, in wrapper fn(*args, **kwargs) File "/usr/local/lib/python2.7/dist-packages/channels/management/commands/runserver.py", line 51, in inner_run http_consumer=self.get_consumer(*args, **options), File "/usr/local/lib/python2.7/dist-packages/channels/management/commands/runserver.py", line 157, in get_consumer return StaticFilesConsumer() File "/usr/local/lib/python2.7/dist-packages/channels/handler.py", line 347, in __init__ self.handler = self.handler_class() File "/usr/local/lib/python2.7/dist-packages/channels/staticfiles.py", line 18, in __init__ super(StaticFilesHandler, self).__init__() File "/usr/local/lib/python2.7/dist-packages/channels/handler.py", line 194, in __init__ self.load_middleware() File "/usr/local/lib/python2.7/dist-packages/django/core/handlers/base.py", line 82, in load_middleware mw_instance = middleware(handler) TypeError: this constructor takes no arguments

kumar303 commented 6 years ago

I guess Django introduced a backwards incompatible change for middleware. That's unfortunate.

edmorley commented 6 years ago

This issue is missing enough detail to tell for sure, but I'm guessing the MIDDLEWARE setting was being used (rather than the old style MIDDLEWARE_CLASSES), which hawkrest doesn't yet support.

Hawkrest will need to be updated to work with Django 1.11's new MIDDLEWARE option, per: https://docs.djangoproject.com/en/1.11/topics/http/middleware/#upgrading-pre-django-1-10-style-middleware

The workaround in the meantime is to use MIDDLEWARE_CLASSES with Django 1.11, which does work.

edmorley commented 6 years ago

One complication is that properly testing this change to hawkrest means overhauling the tests to actually test the middleware end to end, like eg: https://github.com/encode/django-rest-framework/blob/master/tests/test_middleware.py

(And ideally also overhauling the auth parts too, a la https://github.com/encode/django-rest-framework/blob/master/tests/test_authentication.py)

ydaniels commented 6 years ago

@edmorley actually you right am using MIDDLEWARE, thanks I'll make the change you suggested

edmorley commented 6 years ago

@kumar303 I don't suppose you could publish a new version of hawkrest to pick up the fixes for this that were merged, then we can close it out? :-)

kumar303 commented 6 years ago

Hi @edmorley. I'm away on parental leave right now so I won't be able to help on this. Can you email me your PyPI username to kumar.mcmillan@gmail.com? I will add you as a maintainer. Thanks for all your work on mohawk and hawkrest!

edmorley commented 5 years ago

I've just published 1.0.1 which fixes this: https://pypi.org/project/hawkrest/1.0.1/#files https://hawkrest.readthedocs.io/en/latest/#changelog