feat: drift detection and takeover implementation (1/ contd) #1010
Conversation
| wrappedErr := fmt.Errorf("unexpected apply strategy %s is found", applyStrategy.Type) | ||
| _ = controller.NewUnexpectedBehaviorError(wrappedErr) | ||
| return nil, wrappedErr |
There was a problem hiding this comment.
I wonder why don't we return the unexpectedError here but return APIServer errors in other places?
There was a problem hiding this comment.
is that UnExpected erorr are "unExpected" so we don't want to record?
There was a problem hiding this comment.
Hi Ryan! It's still logged; the thing about this one is that the returned error will be formatted into a condition message which the user would be able to see, and the unexpected behavior cannot be handled by the controller part might read a bit weird; do we still want to expose them?
There was a problem hiding this comment.
I will make the changes if that is OK with you.
There was a problem hiding this comment.
I can see pros and cons on either solution
| wrappedErr := fmt.Errorf("unexpected apply strategy %s is found", applyStrategy.Type) | ||
| _ = controller.NewUnexpectedBehaviorError(wrappedErr) | ||
| return nil, wrappedErr |
There was a problem hiding this comment.
I can see pros and cons on either solution
2 participants
Description of your changes
This PR addresses the leftover comments on #985.
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
Special notes for your reviewer
N/A