soynatan / django-easy-audit

Yet another Django audit log app, hopefully the simplest one.
GNU General Public License v3.0
741 stars 182 forks source link

request_started_handler() missing 1 required positional argument: 'environ' #81

Open dopatraman opened 5 years ago

dopatraman commented 5 years ago

When I include easy_audit via the steps in the readme (add it to INSTALLED_APPS and MIDDLEWARE), I get this error on app startup:

request_started_handler() missing 1 required positional argument: 'environ'

As far as I can tell, this argument is supposed to be passed to the signal receiver automatically. I'm not sure what's causing this issue.

jheld commented 5 years ago

Looks like this was before easyaudit upgraded to have django 1.8+ support. If you upgrade to a more recent version I think you'll find that it works.

CharleneCheung commented 5 years ago

import logging from django.core.signals import request_started from easyaudit import settings from easyaudit.signals.request_signals import request_started_handler logger = logging.getLogger(name) """ This file override the signal from channels 2.x. We found a error that when we set DJANGO_EASY_AUDIT_WATCH_REQUEST_EVENTS as True, python pop error as below: request_started_handler() missing 1 required positional argument: 'environ'

In django signal library, it only accept parameter "environ". Moreover, channels pass parameter "scope" instead of "environ".

if you need to use channels as websocket, you must rewrite a new method and assign attribute that easy audit needs.

notes: In channels: signals.request_started.send(sender=self.class, scope=self.scope)

In django signal request_started_handler(sender, environ, **kwargs): """

def ready(): print("game signals ready")

def fixed_request_started_handler(sender, scope, **kwargs):

TODO replicate environ

headers = dict(scope['headers'])
scope['PATH_INFO'] = scope['path']
scope['HTTP_COOKIE'] = headers.get(b'cookie').decode('utf-8')
scope['REQUEST_METHOD'] = scope['method']
scope['QUERY_STRING'] = scope['query_string']
scope['REMOTE_ADDR'] = scope['client'][0]
return request_started_handler(sender, environ=scope, **kwargs)

if settings.WATCH_REQUEST_EVENTS: dispatch_uid = 'easy_audit_signals_request_started' request_started.disconnect(request_started_handler, dispatch_uid=dispatch_uid) request_started.connect(fixed_request_started_handler, dispatch_uid=dispatch_uid)

jheld commented 5 years ago

@CharleneCheung

I'm sorry. When I saw scope I wondered if it was channels related or not, but it didn't occur to me that anyone was using channels and sending these signals. I suppose with the right middleware/signal sending, that's exactly what happens!

OK, so yes, we should be able to handle this. We can try to shove it into our existing request_started_handler, or perhaps make a new one...

I can see how having one request_started handler for WSGI and one for ASGI might be a bit cleaner, but also realistically that would require all downstream project that use both/either to put in that time and effort.

So maybe we have some abstraction code instead, so that WSGI and ASGI contexts just need to return the appropriate info and the handler will just use the data returned from those functions.

I think that might be the better idea. Thoughts?

CharleneCheung commented 5 years ago

yes , for sure.

jheld commented 4 years ago

I've made a branch for this. Untested but based on a closed PR so that helped. I don't know if it works for all ASGI versions or not, yet.

I'll try to get a PR up by end of next week (tested or not).