-
Notifications
You must be signed in to change notification settings - Fork 77
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(agent): Graceful Stop #3219
Conversation
@@ -138,3 +140,7 @@ func (c *Client) getName() (string, error) { | |||
|
|||
return hostname, nil | |||
} | |||
|
|||
func isCancelledError(err error) bool { |
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 didn't find a good way of doing this, the RPC message contains a wrapped error that when I tried to unwrap it was returning a plane interface{}
object 🤷🏾
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.
Did you try:
errors.Is(err, context.Canceled)
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.
Yeah, but it didn't work 😢
if err != nil { | ||
log.Fatal("could not get message from trigger stream: %w", err) | ||
log.Fatal("could not get shutdown listener: %w", err) | ||
} |
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 forcing a shutdown here in case of an error make sense? Something like using: os.Exit(-1)
.
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 believe log.Fatal
is already doing that
// Fatal is equivalent to Print() followed by a call to os.Exit(1).
func Fatal(v ...any) {
std.Output(2, fmt.Sprint(v...))
os.Exit(1)
}
return | ||
} | ||
|
||
if err != nil { | ||
log.Fatal("could not get message from trigger stream: %w", err) | ||
log.Fatal("could not get message from ds connection stream: %w", err) |
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.
log.Fatal("could not get message from ds connection stream: %w", err) | |
log.Fatal("could not get message from data store connection stream: %w", err) |
This PR fixes the problem when trying to stop the agent from the CLI option
Changes
Fixes
Checklist
Loom video
https://www.loom.com/share/d0bf6ba20a074b16811e5524b20bb36f