search

aschn / drf-tracking

Utils to track requests to Django Rest Framework API views
http://drf-tracking.readthedocs.org/
ISC License
366 stars 95 forks source link

Log traceback errors on model #18

Closed aschn closed 7 years ago

aschn commented 8 years ago

Fixed issue #16

This adds an errors field, and sets it by overwriting the handle_exception method on the view mixin. The response body is unaffected.

coveralls commented 8 years ago

Coverage Status

Changes Unknown when pulling be1fe2b699647ae2be465f1d864f0061f2864481 on handle_500 into \ on master**.

triat commented 7 years ago

Hey, thanks for the fix. Is it going to be merged soon ?

avelis commented 7 years ago

@triat It most certainly can. I believe @aschn wanted to be sure this PR actually worked for issue #16 before merging. If it does then I can merge it in. However, that was certainly a long time ago.

triat commented 7 years ago

Sure I just wanted to know because we had the error yesterday with a malformation of a JSON causing a 500.

triat commented 7 years ago

@avelis Can I help you on this one ? as we got the errors too.

avelis commented 7 years ago

@triat By all means. All contributions are appreciated.

triat commented 7 years ago

Hey @avelis, I took the branch on local and tried to pass the tests and they are working on my machine. Could it be possible to rebase the branch from master ? Thanks

edit: ok nvm, I've reproduce the errors (finally got a working tox installation)

triat commented 7 years ago

Hey @avelis, do you need an other PR to fix the conflicts or you can do it ? Thanks

avelis commented 7 years ago

@triat probably need a PR to fix this. Been really tied up at work as of late

triat commented 7 years ago

Sure, no problem @avelis. I just created a new PR as I can't fix the conflict here. Hope it helps you. Thanks for your reactivity

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.003%) to 40.768% when pulling b27bc96d26af3ee3293174905b80dfb55a2d8463 on handle_500 into d4cf08f3040b452e336b561321d0e66f587b06d7 on master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.003%) to 40.768% when pulling b27bc96d26af3ee3293174905b80dfb55a2d8463 on handle_500 into d4cf08f3040b452e336b561321d0e66f587b06d7 on master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.003%) to 40.768% when pulling b27bc96d26af3ee3293174905b80dfb55a2d8463 on handle_500 into d4cf08f3040b452e336b561321d0e66f587b06d7 on master.

triat commented 7 years ago

About those tests, I don't know if we should have a look or if it's normal that they are slow sometimes ?

avelis commented 7 years ago

@triat There is a couple of things we could do. We could simply re-teach to the test to the new time. 42 ms I believe. We could also use flaky but I doubt that will lower the time for those scenario's. Thoughts?

triat commented 7 years ago

@avelis I'm not a huge fan of changing the limit time, if your goal is to have an API with a response time with max 200 ms, having drf-tracking take already almost 25% of your response time.

Something that we could do to reduce this amount to 0 is to start using the async in python 3 and / or try to do something similar for python 2 (config with celery?)

avelis commented 7 years ago

@triat Not against Python 3 async but am concerned with Python 2 and Celery. I like Celery to be honest but am concerned forcing people to use that technology to reduce the middleware's overhead.

triat commented 7 years ago

@avelis I'm not talking about forcing them put maybe propose a configuration working with celery if people really do care about their API response time. The other solution in Python 2 multithreading but from what I heard, it's far from being simple to use

triat commented 7 years ago

@avelis Hey, can we check now if the tests pass here too by rebasing on master? Thanks

avelis commented 7 years ago

@triat I was able to do merge via the Github Desktop UI. We can now observe if the checks pass.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.03%) to 40.882% when pulling 597585aac9458312117f0c1edceb50829bcc9e8f on handle_500 into b767aa03af452146eb4e777b2b6fef09b1f78093 on master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.03%) to 40.882% when pulling 597585aac9458312117f0c1edceb50829bcc9e8f on handle_500 into b767aa03af452146eb4e777b2b6fef09b1f78093 on master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.03%) to 40.882% when pulling 597585aac9458312117f0c1edceb50829bcc9e8f on handle_500 into b767aa03af452146eb4e777b2b6fef09b1f78093 on master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.03%) to 40.882% when pulling 597585aac9458312117f0c1edceb50829bcc9e8f on handle_500 into b767aa03af452146eb4e777b2b6fef09b1f78093 on master.

triat commented 7 years ago

@avelis apparently it does 👍

triat commented 7 years ago

Hey @avelis, can I just ask you, when you have time, to trigger the build for pipy :) thanks a lot