-
Notifications
You must be signed in to change notification settings - Fork 143
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
Diagnostics creates the necessary folder for -f argument #3340
Diagnostics creates the necessary folder for -f argument #3340
Conversation
Pinging @elastic/elastic-agent (Team:Elastic-Agent) |
- makes diagnostics command to create the necessary folders when a custom path is provided by the '-f' flag. - refactors the diagnosticCmd splitting it into smaller functions - add test createFile - rename logs_test.go createFile function to createFileEmpty so its name wont collide with the newly created 'createFile' function. add license headaer
26f6494
to
cdcd1eb
Compare
🌐 Coverage report
|
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.
Just a small comment about the renaming for the logs, looks ok otherwise
I saw that the PR is labeled for 8.10 backport but we are already creating BCs for that release and this is not a blocker for 8.10. Do we want to backport it anyways ?
@@ -99,7 +99,7 @@ func TestGetLogFilenames(t *testing.T) { | |||
|
|||
t.Run("does not return non-log entries", func(t *testing.T) { | |||
dir := t.TempDir() | |||
createFile(t, dir, "excluded") | |||
createFileEmpty(t, dir, "excluded") |
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.
Just checking: has this been renamed because of name collision within the same package?
Failing tests are unrelated. I'm forcing the merge. |
- makes diagnostics command to create the necessary folders when a custom path is provided by the '-f' flag. - refactors the diagnosticCmd splitting it into smaller functions - add test createFile - rename logs_test.go createFile function to createFileEmpty so its name wont collide with the newly created 'createFile' function. add license headaer (cherry picked from commit 4cd83bf)
- makes diagnostics command to create the necessary folders when a custom path is provided by the '-f' flag. - refactors the diagnosticCmd splitting it into smaller functions - add test createFile - rename logs_test.go createFile function to createFileEmpty so its name wont collide with the newly created 'createFile' function. add license headaer (cherry picked from commit 4cd83bf) Co-authored-by: Anderson Queiroz <[email protected]>
- makes diagnostics command to create the necessary folders when a custom path is provided by the '-f' flag. - refactors the diagnosticCmd splitting it into smaller functions - add test createFile - rename logs_test.go createFile function to createFileEmpty so its name wont collide with the newly created 'createFile' function. add license headaer
What does this PR do?
Fixes how the diagnostics command handles the custom path to save the diagnostics. Right now if the custom file path to save the diagnostics on contains folders that do not exist, the diagnostics fail to create the file, consequently failing to save the diagnostics.
Why is it important?
It not clear on the diagnostics help the path to the file must exist, it'd be much easier for our users if the diagnostics is capable of creating the necessary folders.
Checklist
[ ] 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 in./changelog/fragments
using the changelog tool[ ] I have added an integration test or an E2E testHow to test this PR locally
Run the diagnostics command with the
-f
flag, passing a path where at least one folder does not exist. It will fail on main and succeed with this change.E.g.:
Related issues
Logs
Questions to ask yourself