Skip to content
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

Add file output to foreach command #141

Merged
merged 5 commits into from
Aug 5, 2024
Merged

Add file output to foreach command #141

merged 5 commits into from
Aug 5, 2024

Conversation

rnorth
Copy link
Collaborator

@rnorth rnorth commented Jul 23, 2024

Fixes #7

When running turbolift foreach there are several frequent use cases that are ill catered for:

  • reviewing the logs on a per-repo basis - logs are currently in one giant scrollback
  • reviewing just failure or just success logs
  • (very often) gathering the names of the repos where the command succeeded or failed - for example, when running foreach to run tests

To avoid further cluttering the terminal output, this PR:

  • Creates a temp directory each time it's run, which will likely exist until system reboot
  • In that directory creates the following structure, where org/repo/logs.txt is repeated for every repository (mirroring the structure of the work directory):
some-temp-dir
   \ successful
       \ repos.txt        # a list of repos where the command succeeded
       \ org
           \ repo
               \ logs.txt # logs from the specific foreach execution on this repo
           ....
   \ failed
       \ repos.txt        # a list of repos where the command succeeded
       \ org
           \ repo
               \ logs.txt # logs from the specific foreach execution on this repo
           ....

Some notable points:

  • the emitted repos.txt files are suitable for replay back into turbolift using the -r option
  • we don't stop the current logging to terminal output - this is mainly for backwards compatibility

cmd/foreach/foreach.go Outdated Show resolved Hide resolved
cmd/foreach/foreach.go Outdated Show resolved Hide resolved
@sledigabel
Copy link
Contributor

Great addition, nice one @rnorth !
It will also fix #7 and help with #31

Dan7-7-7
Dan7-7-7 previously approved these changes Jul 23, 2024
Copy link
Collaborator

@Dan7-7-7 Dan7-7-7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great 👍 the file structure is a nice idea, I was assuming it would be a single file containing all the failed logs together, but this is clearer/ easier to navigate.

Might be an idea to explicitly provide the command to run foreach against only the repos that failed, in case people don't realise that's possible (or even have a --failed flag that does this automatically?) but perhaps not necessary.

cmd/foreach/foreach.go Outdated Show resolved Hide resolved
@sledigabel
Copy link
Contributor

In terms of naming, I was thinking it could be worth adding some sort of timestamp after the repos.txt.
The reason is if you use it to replay the process will overwrite the file as it is being read.

Otherwise we need to make sure we tell users to copy the failed/repos.txt before using it in a foreach loop again.

@rnorth
Copy link
Collaborator Author

rnorth commented Aug 5, 2024

Thanks for the review, I'd like to merge and release today as there's demand to use this internally.

@sledigabel to answer this:

In terms of naming, I was thinking it could be worth adding some sort of timestamp after the repos.txt.
The reason is if you use it to replay the process will overwrite the file as it is being read.

The output repos.txt files also go into a temp output directory, so they don't overwrite one another or overwrite the one in the campaign directory. My assumption is that, if a user wants to do something with lists of successful/failed repos, they can copy the contents and do whatever they want.

@Dan7-7-7

Might be an idea to explicitly provide the command to run foreach against only the repos that failed, in case people don't realise that's possible (or even have a --failed flag that does this automatically?) but perhaps not necessary.

I like that - it's a really good idea. My initial thought was "how do we know which the previous run was" but if we were to combine this with @sledigabel's suggestion of timestamped output directories I can see how it could work. I will raise a separate issue, though, as it feels like it would be somewhat complex.

@rnorth rnorth enabled auto-merge (squash) August 5, 2024 07:47
@rnorth rnorth merged commit b5cd6d2 into main Aug 5, 2024
4 checks passed
@rnorth rnorth deleted the foreach-logs-to-files branch August 5, 2024 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collect errors and display nicely
3 participants