-
Notifications
You must be signed in to change notification settings - Fork 68
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
Few fixes of checking exceptions #107
Conversation
titusjaka
commented
Aug 13, 2018
- since now we don't want to notify user if something goes wrong. So, I remove checkData.State = EXCEPTION
- if there are 0 metrics in trigger, we want to switch trigger in TTLState instead of NODATA
- since now we don't want to notify user if something goes wrong. So, I remove checkData.State = EXCEPTION - if there are 0 metrics in trigger, we want to switch trigger in TTLState instead of NODATA
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.
Смотри комменты, там кажется можно еще немного прибраться
return checkData, nil | ||
} | ||
case ErrTriggerHasOnlyWildcards: | ||
case ErrTriggerHasNoTimeSeries, ErrTriggerHasOnlyWildcards: |
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.
Ох, ты тут что-то опасное пытаешься сделать.
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.
Но вроде все норм)
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.
Я покопался в логике. Они очень похожие, поэтому я объединил.
checker/check.go
Outdated
return checkData, nil | ||
} | ||
case ErrTriggerHasOnlyWildcards: | ||
case ErrTriggerHasNoTimeSeries, ErrTriggerHasOnlyWildcards: | ||
triggerChecker.Logger.Debugf("Trigger %s: %s", triggerChecker.TriggerID, checkingError.Error()) | ||
triggerState := ToTriggerState(triggerChecker.ttlState) | ||
if len(checkData.Metrics) == 0 && triggerState != OK { |
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.
triggerState != OK не хотим сразу убрать? Это условие нужно тут теперь вообще?
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.
len(checkData.Metrics) == 0 Точно нужно, ибо мы реально хотим кидать эту ошибку только тогда, когда метрик еще не было
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.
с != OK согласен
99986fc
to
079605f
Compare
@borovskyav вливаю? |
На стейджинге я бы это глянул |
Ок, завтра катну |
Кажется, всё работает как надо. |
Ничосе, аж дважды заапрувил =) |