-
Notifications
You must be signed in to change notification settings - Fork 197
mage: Improve error messages and resource handling #10446
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
base: main
Are you sure you want to change the base?
Conversation
This commit includes minor developer tooling improvements: - Improves the checksum mismatch error message for better diagnostics. - Fix spurious "error closing" logs when double-closing files explicitely and with `defer`
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
|
||
func closeOrLog(closer io.Closer, what string) { | ||
err := closer.Close() | ||
if err == nil || errors.Is(err, os.ErrClosed) { |
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.
This is a change in behavior: you are explicitly ignoring os.ErrClosed
. Wouldn't it be better to figure out where we are closing the same io.Closer
twice ?
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.
That can be done as well but I figured cleanliness is slightly preferred over having to set nil to the file pointers, at least for this part of the code. Other parts don't even log the Close
errors.
💚 Build Succeeded
History
cc @orestisfl |
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
What does this PR do?
This commit includes minor developer tooling improvements:
defer
Why is it important?
Checklist
./changelog/fragments
using the changelog tool