Skip to content
This repository was archived by the owner on Sep 26, 2023. It is now read-only.

[WIP] Enable/Disable service notifications#29

Open
takus wants to merge 3 commits into
dmytro:masterfrom
takus:feature/notification_command
Open

[WIP] Enable/Disable service notifications#29
takus wants to merge 3 commits into
dmytro:masterfrom
takus:feature/notification_command

Conversation

@takus

@takus takus commented Jun 13, 2014

Copy link
Copy Markdown

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.

@dmytro

dmytro commented Jun 13, 2014

Copy link
Copy Markdown
Owner

@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.

@takus

takus commented Jun 13, 2014

Copy link
Copy Markdown
Author

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

@takus

takus commented Jun 13, 2014

Copy link
Copy Markdown
Author

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

@dmytro

dmytro commented Dec 25, 2014

Copy link
Copy Markdown
Owner

@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

dmytro commented Dec 25, 2014

Copy link
Copy Markdown
Owner

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants