honeybadger-io / honeybadger-python

Send Python and Django errors to Honeybadger.
https://www.honeybadger.io/
MIT License
15 stars 26 forks source link

Plugin support #28

Closed ifoukarakis closed 6 years ago

ifoukarakis commented 6 years ago

This PR is a suggestion for refactoring according to issue #27. It includes the following changes:

This PR should have no effect on end-users of the library. However it can be used as the basis for adding payload generation for other projects as well.

TBH, I'm not an expert in Django, so any feedback on unittests are more than welcomed!

Finally, there's a point I'd like to discuss. Django middleware caches . With the plugin mechanism in place, DjangoPlugin class might be a better candidate for moving this logic. This way Honeybadger may become agnostic to any frameworks.

ifoukarakis commented 6 years ago

Please note that I also had to make an ugly fix for testfixtures to work with Python 3.2

stympy commented 6 years ago

This is looking good to me. I'm fine with moving the current request logic to the DjangoPlugin class.

Did your original PR comment get cut off? I'm not clear on what you meant by the "open discussion" comment.

ifoukarakis commented 6 years ago

Hey, seems that the last part actually got cut off. My comment was that request is Django specific. So it's lifecycle management may be more suitable in Django middleware.

stympy commented 6 years ago

Agreed. We've tried to isolate the Rails-related stuff as much as possible in the Ruby gem, so I see no reason not to the same for Django-related stuff here.

ifoukarakis commented 6 years ago

Ok, I'll update current PR as soon as I find some time:)

ifoukarakis commented 6 years ago

Hi,

Finally managed to find some time and update the PR according to the last comment. I also included an example Django app I used to test the changes.

stympy commented 6 years ago

Awesome, thanks!