nytimes / collectd-rabbitmq

A collected plugin, written in python, to collect statistics from RabbitMQ.
https://collectd-rabbitmq.readthedocs.org/
Other
145 stars 79 forks source link

Fix - plugin crash when metric is not in collectd types db #56

Closed caduri closed 7 years ago

caduri commented 7 years ago

When a metric is missing from the types db the plugin crushes. In this fix I catch the exception and just notify collectd, basically ignoring the metric

what do you think?

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.6%) to 99.383% when pulling 75066a75b5a7e66cd287a1d3235a71ead4c6b5f2 on caduri:master into de27702750b26c4d8909c1b1e299ac2e30f85db5 on NYTimes:master.

jimbydamonk commented 7 years ago

This looks good. Do you think you could add a test to cover when a exception gets thrown during dispatch ? Thanks.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling a01ca4dfc985feaf0917aae88bb6927941df6f7d on caduri:master into de27702750b26c4d8909c1b1e299ac2e30f85db5 on NYTimes:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling ef9ff0f033361889811bc630074b28bd4a210fcd on caduri:master into de27702750b26c4d8909c1b1e299ac2e30f85db5 on NYTimes:master.

caduri commented 7 years ago

@jimbydamonk I added a test. LMK if you have any other requests

caduri commented 7 years ago

@jimbydamonk when are you merging this PR?

jimbydamonk commented 7 years ago

It's now in version 1.18.0 Thanks!

caduri commented 7 years ago

@jimbydamonk the version in pypi is still 1.17.0, can you take a look?

jimbydamonk commented 7 years ago

I pushed out the new version a few days ago. Thanks for reminding me.

richarson commented 7 years ago

Just curious: you don't publish/tag new releases here on github anymore?

jimbydamonk commented 7 years ago

Oh, I should. I just didn't push my tags. :-) I am going to fix the pipeline so that this is all automatic. Thanks for calling me out.

richarson commented 7 years ago

No problem, I just found it odd :)

And thank you! Since I use github's RSS feed to find out about new releases of the repos I follow (e.g. https://github.com/NYTimes/collectd-rabbitmq/releases.atom), now I don't have to track a different source for yours :)