Skip to content
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

message_template ignored if check has a notification #70

Open
stewartwebb opened this issue Nov 9, 2018 · 1 comment
Open

message_template ignored if check has a notification #70

stewartwebb opened this issue Nov 9, 2018 · 1 comment

Comments

@stewartwebb
Copy link

I'm trying to use a message_template. On upgrade from 1.0.0 to 1.1.1, some of my slack messages were "truncated". It was only showing the notification. I investigated and I think it is due to a fix in 1.1.0: #25. I set the notification field in some my checks and in the event where it is set the handler is ignoring my message_template in favour of the raw notification. I think the fix was correct but I'm not sure if the behaviour is correct?

The line that I think is suspicious.

https://github.com/dunpyl/sensu-plugins-slack/blob/c527e90c319320c69dc786b8a7204b0c9da736fe/bin/handler-slack.rb#L89-L96

How the logic should work in my head is something like this:

if payload_template {
  // use that
} else {
  if message_template {
    // Use message_template
  } else if @event['check']['notification'] {
    // Use notification
  } else {
    // Construct with generated bits
  }
}
@majormoses
Copy link
Member

I think that makes sense, this would technically be a breaking change and would need to be done as a major release; which is perfectly ok we just want to avoid surprises for people in minor release versions. We follow semver and a major vs a minor bump indicates nothing more than there was a breaking change. It is meant to convey the safety of upgrades to the consumer. I would try to look at making the changes look something like this:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants