search

eht16 / python-logstash-async

Python logging handler for sending log events asynchronously to Logstash.
MIT License
186 stars 51 forks source link

python3 cross compatibility and django=1.8 #3

Closed ornoone closed 7 years ago

ornoone commented 7 years ago

hi, i am experiencing some bug while using the DjangoLogstashFormatter with django 1.8 and python 3.4.

it seem that this lib is not compatible with either one by 2 at least two lines :

logstash_async/formatter.py:235 extra_fields['req_uri'] = request.get_raw_uri()

but django WSGIRequest object has no get_raw_uri() in version 1.8 (LTS). I propose to skip this extra_fields for later versions.

logstash_async/formatter.py:236: extra_fields['req_user'] = unicode(request.user) if request.user else ''

you use unicode, which is a 2.7 only string type. you can either import six or do a try: except ImportError for string types to be cross compatible.

this is the only 2 things a saw for now, but I will keep investigating.

it is a best practice to support from current LTS to current stable for your app. that's why I speak about 1.8... https://www.djangoproject.com/download/#supported-versions

If you want to be python3 compatible and LTS compatible, I can eagerly do a PR to fix all thing a found and build some unittests to check it all.

eht16 commented 7 years ago

A PR is very welcome. I usually just test and use against Python2 and Django 1.10, to be honest. While properly supporting Python3 I fully support, I don't feel so strong about supporting Django 1.8 but since it is probably rather easy, why not.

Tests are on my TODO list but so far I didn't get it to yet.

eht16 commented 7 years ago

request.user is now checked for existence before being accessed and I replaced unicode by text_type from the six library. So both issues should be fixed.

I still plan to add unittests at some point.