New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add handle_silenced parameter to handler defined type #753
Conversation
@@ -177,4 +177,9 @@ | |||
it { should contain_sensu_handler('myhandler').with_handle_flapping( true ) } | |||
end | |||
|
|||
context 'handle_silenced' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we also need a test for when this is false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added another spec to confirm that handle_silenced
when it's left as the default of false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've now also added a spec to check when handle_silenced
is explicitly set to false
@@ -67,6 +67,11 @@ | |||
# Default: false. | |||
# Valid values: true, false | |||
# | |||
# [*handle_silenced*] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add an example to https://github.com/sensu/sensu-puppet/blob/master/tests/sensu-server.pp and document which file should be modified what json should be expected. That way we can functionally test this behaves as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added that in d05b1c7, wasn't quite sure how to go about it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
@madAndroid Awesome work, thanks for adding this! |
@ghoneycutt - thanks :) Comments addressed and changes made |
spec/defines/sensu_handler_spec.rb
Outdated
it { should contain_sensu_handler('myhandler').with_handle_silenced( true ) } | ||
end | ||
|
||
context 'handle_silenced set to default' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the default behavior, which is great. We also need to explicitly test for setting it to false like you do with the test above, setting it to true.
@@ -67,6 +67,11 @@ | |||
# Default: false. | |||
# Valid values: true, false | |||
# | |||
# [*handle_silenced*] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
@ghoneycutt - Comments addressed, and all tests now passing :) |
Sorry to ask you to change some of the things I asked here for, like type validation, though we've implemented data types and all PR's will now need to be rebased. |
a4f603e
to
2f12bcc
Compare
@ghoneycutt - No worries, I've rebased, should be ready to go now |
Thanks for adding this new functionality! Released in v2.28.0 |
@ghoneycutt - Thanks! :) |
We have a handler that needs to operate on events, regardless of whether they've been silenced or not - this change implements the
handle_silenced
handler config option, as outlined in https://sensuapp.org/docs/0.29/reference/handlers.html