Refactor: Move inTimeIntervals from notify to timeinterval
#3556
Conversation
Signed-off-by: gotjosh <[email protected]>
Signed-off-by: gotjosh <[email protected]>
| @@ -478,6 +478,8 @@ func run() int { | |||
| timeIntervals[ti.Name] = ti.TimeIntervals | |||
| } | |||
|
|
|||
| intervener := timeinterval.NewIntervener(timeIntervals) | |||
There was a problem hiding this comment.
I started moving the loop above into timeinterval as well but quickly realised that we have an import problem - this is part of a larger problem and I'll address it in a separate PR.
I don't understand why the marshalling of config -> struct happens with structs that live in timeinterval when this is generally done as part of theconfig package.
| if len(muteTimeIntervalNames) == 0 { | ||
| return ctx, alerts, nil | ||
| } |
There was a problem hiding this comment.
This guard clause was missing from this path.
Signed-off-by: gotjosh <[email protected]>
Signed-off-by: gotjosh <[email protected]>
| @@ -659,3 +660,97 @@ func mustLoadLocation(name string) *time.Location { | |||
| } | |||
| return loc | |||
| } | |||
|
|
|||
| func TestIntervener_Mutes(t *testing.T) { | |||
There was a problem hiding this comment.
These tests are technically a duplicate of what we already have in notify - I think they have some merit in being here as there might be more we want to test as part of this interface in the future (e.g. when we introduce the marker) and it gave me more confidence in the refactor.
…metheus#3556) * Refactor: Move `inTimeIntervals` from `notify` to `timeinterval` There's absolutely no change of functionality here and I've expanded coverage for similar logic in both places. --------- Signed-off-by: gotjosh <[email protected]>
This should make implementing #3520 a bit cleaner given that we shouldn't need to introduce a direct dependency on marker in
notify.There's absolutely no change of functionality here and I've expanded coverage for similar logic in both places.