fix: topic-handler duration() -> alertDuration() #2448
Conversation
| @@ -2,6 +2,10 @@ | |||
|
|
|||
| ## Unreleased | |||
|
|
|||
| ### Bugfixes | |||
| - [#2448](https://github.com/influxdata/kapacitor/pull/2448): Changes the alert-handler match function duration() to be alertDuration() to avoid name collision with the type conversion function of the same name. | |||
There was a problem hiding this comment.
Is this something we can change in a patch release? Or is it not really a breaking change because the collision was preventing all uses of the match function form?
There was a problem hiding this comment.
Yah, it isn't a breaking change because the old duration() function never actually worked.
| if spec.Match != "" { | ||
| // Wrap handler in match handler | ||
| handlerDiag := s.diag.WithHandlerContext(ctx...) | ||
| h, err = newMatchHandler(spec.Match, h, handlerDiag) | ||
| if h == nil { | ||
| panic("handler is nil, this should not happen") |
There was a problem hiding this comment.
Does this one need to be a panic? Could we return an err instead?
There was a problem hiding this comment.
It is a state that should never happen and should panic. The problem was it was panic-ing in a very distant place and made it hard to debug.
services/alert/handlers.go
Outdated
|
|
||
| if h.h == nil { | ||
| panic("h.h is nil") | ||
| } |
There was a problem hiding this comment.
This looks like a debug check, do we want to leave it in?
There was a problem hiding this comment.
Doh, I thought I removed it.
the topic handler match function
duration()was colliding with the lambda function type conversion functionduration. This removes that collision by changing the name of the topic handler match function toalertDuration.