-
Notifications
You must be signed in to change notification settings - Fork 246
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: make lease functions less chatty #4100
Conversation
@@ -536,13 +537,7 @@ func (lm *leasableMachine) ReleaseLease(ctx context.Context) error { | |||
defer cancel() | |||
} | |||
|
|||
err := lm.flapsClient.ReleaseLease(ctx, lm.machine.ID, nonce) | |||
contextTimedOutOrCanceled := errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) | |||
if err != nil && (!contextWasAlreadyCanceled || !contextTimedOutOrCanceled) { |
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.
does it make any difference for contextWasAlreadyCanceled
and !contextTimedOutOrCanceled
?
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.
Good catch! There are places where we call ReleaseLease and not logging any errors from that. Let me adjust them.
Adding releaseLease() function to log the error from |
Without this change, the console would be filled with "failed to release lease: lease not found" even if flyctl couldn't acquire one lease. ``` Updating existing machines in '...' with bluegreen strategy WARN failed to acquire lease: failed to get lease on VM e7843075ad2408: request returned non-2xx status: 408: WARN failed to release lease for machine 178122df9672d8: lease not found WARN failed to release lease: lease not found WARN failed to release lease for machine 7811772a5d7698: lease not found WARN failed to release lease: lease not found WARN failed to release lease for machine e286076f933508: lease not found WARN failed to release lease: lease not found ... ```
Without this change, the console would be filled with "failed to release lease: lease not found" even if flyctl couldn't acquire one lease.
Change Summary
What and Why:
How:
Related to:
Documentation