Error reporter #261
Error reporter #261
Conversation
…n seperate repos. start of a generic error handling implementation
Codecov Report
@@ Coverage Diff @@
## master #261 +/- ##
==========================================
- Coverage 93.2% 90.49% -2.72%
==========================================
Files 41 41
Lines 2724 2735 +11
==========================================
- Hits 2539 2475 -64
- Misses 185 260 +75
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #261 +/- ##
==========================================
- Coverage 93.2% 90.24% -2.97%
==========================================
Files 41 41
Lines 2724 2748 +24
==========================================
- Hits 2539 2480 -59
- Misses 185 268 +83
Continue to review full report at Codecov.
|
|
This looks great! 👍 |
|
|
||
| Error reporting plugins register a class which implements a ``report`` method, which is invoked when a Django Q cluster encounters an error, pasing information to the particular service. Error reporters must be :ref:`configured<error_reporter>` via the ``Q_CLUSTER`` dictionary in your :file:`settings.py`. These settings are passed as kwargs upon initiation of the Error Reporter. Therefore, in order to implement a new plugin, a package must expose a class which will be instantiated with the necessary information via the ``Q_CLUSTER`` settings and implements a single ``report`` method. | ||
|
|
||
| For example implementations, see `django-q-rollbar <https://github.com/danielwelch/django-q-rollbar>` and `django-q-sentry <https://github.com/danielwelch/django-q-sentry>` |
There was a problem hiding this comment.
I like that you provide examples except that they are references to a different repository.
Shouldn't django-q-rollbar be included by default in this repo?
There was a problem hiding this comment.
Any idea's on how you would like to prevent this change from breaking everybody's current setup?
Understandable. To me, the most obvious and probably easiest solution is just to leave the old rollbar support in place along side the error reporter implementation. Then no one currently using rollbar support needs to modify their Q_CLUSTER settings, while new projects can instead use the error_reporter config setting and include the django-q-rollbar extra. Seems to me this would involve simply "putting back" what was removed from django_q/conf.py and django_q/cluster.py. I could modify the documentation changes to re-include the rollbar section, maybe with a note about that config setting being there for compatibility purposes and a link to the error_reporter setting. Maybe a deprecation warning if `Q_CLUSTER['rollbar'] is detected, if you're inclined.
I like that you provide examples except that they are references to a different repository.
Shouldn't django-q-rollbar be included by default in this repo?
In my opinion, different repositories is a strength (and kind of the point). I don't see any reason that django-q-rollbar should be included by default over, let's say, django-q-sentry aside from the backwards compatibility issue you brought up, which I think is valid. So the question seems to be, what is the best way to provide rollbar support for current users without breaking things: via Q_CLUSTER['rollbar']" vs. including django-q-rollbar in the django-q repository itself. To me, it seems cleaner to provide django-q-rollbar in the same way that django-q-sentry and potentially other plugins are provided: via a seperate repository. But I am but a hopeful contributor and those design decisions are probably not mine to make.
|
@Eagllus I went ahead and implemented the suggestion I made above and modified documentation as well. This should prevent anything from breaking for those already using the |
|
Looks great @danielwelch . I agree that rollbar does not need to be included by default. |
|
I merged the code to master and fixed some of links in the sphinx documentation. Looks good. |
implementing pluggable error reporters, as discussed in #210.
I've proposed some implementations compatible with this for rollbar and sentry with some documentation available in each repository's README.md.
Includes documentation changes for configuring error reporters.