-
Notifications
You must be signed in to change notification settings - Fork 20
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
fix: mysql error alerts when the auto ops rule is already triggered #196
Conversation
@@ -836,6 +833,13 @@ func (s *AutoOpsService) ExecuteAutoOps( | |||
if err := s.validateExecuteAutoOpsRequest(req, localizer); err != nil { | |||
return nil, err | |||
} | |||
triggered, err := s.checkIfHasAlreadyTriggered(ctx, localizer, req.Id, req.EnvironmentNamespace) |
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 had to move this implementation from the transaction to check if it had already been triggered because the response returns the AlreadyTriggered
boolean.
handler := command.NewAutoOpsCommandHandler(editor, autoOpsRule, s.publisher, req.EnvironmentNamespace) | ||
if err := handler.Handle(ctx, req.ChangeAutoOpsRuleTriggeredAtCommand); err != nil { | ||
return err | ||
} | ||
if err = autoOpsRuleStorage.UpdateAutoOpsRule(ctx, autoOpsRule, req.EnvironmentNamespace); err != nil { | ||
if err == v2as.ErrAutoOpsRuleUnexpectedAffectedRows { |
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.
If there are no affected rows, it will not treat as an error. Otherwise, it might trigger the alert.
logger.Warn( | ||
"Failed to disable feature", | ||
if err != nil { | ||
if code := status.Code(err); code == codes.FailedPrecondition { |
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 split the implementation to print warning and error levels to make debugging easier.
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.
lgtm!
If the user schedule to turn on a flag, and before the auto ops run, he enables the flag, it will treat it as an error, so I'm changing the ExecuteOperation to return nil if the operation is already done.
Things done