-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Update imports to elastic-agent-libs/file
where needed
#31419
Conversation
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
💔 Build Failed
Expand to view the summary
Build stats
Test stats 🧪
Steps errorsExpand to view the steps failures
|
This pull request is now in conflicts. Could you fix it? 🙏
|
elasit-agent-libs/file
where neededelastic-agent-libs/file
where needed
This pull request is now in conflicts. Could you fix it? 🙏
|
base64Encoder.Close() | ||
f.Sync() | ||
_ = f.Sync() |
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.
Can we at least return these errors, I know it was not there before but should we suppress them in this function? This does not feel write, I think it was a bug.
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.
f.Sync
errors are not returned on purpose. Those might fail sometimes, but it is not an error from Beats point of view. Also, if the write calls return an error it is not an issue, because Beat saves the files constantly, so we are not losing data/information.
Unfortunately, when we touch the disk things can go wrong more times than we think. So we have resilience at strategically important file writes (see. SafeFileRotate
). But not all Write
call has to be checked.
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.
Also, if we happened to expose a non-error "error" I do not want to track down what changed... I would rather focus on the actual goal of my project: moving stuff out of libbeat. The scope of this PR is already too big thanks to the linter.
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.
@kvch could you please turn your reply into a comment before those suppressed errors? otherwise it looks strange that everyone is fine with it.
What does this PR do?
This PR removes some parts of
libbeat/common/file
package that was moved to elastic-agent-libs.Why is it important?
We avoid maintaining the same code twice in different repositories.
Checklist
- [ ] I have commented my code, particularly in hard-to-understand areas- [ ] I have made corresponding changes to the documentation- [ ] I have made corresponding change to the default configuration files- [ ] I have added tests that prove my fix is effective or that my feature works- [ ] I have added an entry inCHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.