nagira icon indicating copy to clipboard operation
nagira copied to clipboard

[WIP] Enable/Disable service notifications

Open takus opened this issue 10 years ago • 5 comments

Hi, i'm a user of this awesome product, and use it for our production systems.

I'd like to enable/disable service notification, and I tried to implement new feature in this PR. After merging this, we can enable/disable monitor like this:

$ curl -X PUT 'http://example.com:4567/_notifications/host01/DISK' -d '{"command":"enable"}'
{"result":true,"object":[{"data":{"host_name":"host01","service_description":"DISK","action":"ENABLE_SVC_NOTIFICATIONS"},"result":true,"messages":[]}]}

$ curl -X PUT 'http://example.com:4567/_notifications/host01/DISK' -d '{"command":"disable"}'
{"result":true,"object":[{"data":{"host_name":"host01","service_description":"DISK","action":"DISABLE_SVC_NOTIFICATIONS"},"result":true,"messages":[]}]}

But I'm not sure whether this URI is proper or not. Could you tell me your thought? In addition, if I implement endpoint for other operation (e.g. enable/disable host notification), how should I implement it?

Of course, I'm ready for editing this PR. Thanks in advance.

takus avatar Jun 13 '14 09:06 takus

@takus Thank you very much for the submission. I will review ASAP.

Could you please take a look at the Travis errors in the meantime, please.

dmytro avatar Jun 13 '14 09:06 dmytro

Sorry, I'm a newbie of ruby/sinatra. I'll fix it soon.

takus avatar Jun 13 '14 09:06 takus

@dmytro it seems that there are some pending tests. Should I fix it?

takus avatar Jun 13 '14 11:06 takus

@takus Hi, apologies for the confusion with the errors. Those were because of the changes in RSpec, I've fixed them already.

As for the URL's of the endpoints: I think it would be better to 'hang' notification routes from host and services. Like:

PUT /_status/:host_name/_services/:service_description/_notifications/

and

PUT /_status/:host_name/_notifications

I've put simple guideline related to this: for the consistency of next additions. Please look at the docs/API_NAMESPACING.md file.

Let me know if it is not clear.

dmytro avatar Dec 25 '14 05:12 dmytro

Please also be aware, there are some directory layout changes: ./lib/app now moved to ./app

dmytro avatar Dec 25 '14 05:12 dmytro