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 --quiet or --no-progress-bars to spfs commit and push #1022

Open
dcookspi opened this issue May 11, 2024 · 4 comments · Fixed by #1068
Open

Add --quiet or --no-progress-bars to spfs commit and push #1022

dcookspi opened this issue May 11, 2024 · 4 comments · Fixed by #1068
Assignees
Labels
agenda item Items to be brought up at the next dev meeting enhancement New feature or request SPI AOI Area of interest for SPI SPI-0.41

Comments

@dcookspi
Copy link
Collaborator

dcookspi commented May 11, 2024

Is your feature request related to a problem? Please describe.

Some spfs commands use console/text based progress bars. The progress bars seem, to clear the console as a side-effect. spfs commit and spfs push are examples of where this happens.

Users have found, when those commands are used in scripts, either directly or indirectly (e.g. from an application launcher, or renderfarm job launcher), useful output from other parts of the script is lost. It is cleared away by the console clearing that happens as part of the progress bar display. Losing key parts of their script's output annoying and ideally they'd like it to stop, or have an option stop it happening.

Describe the solution you'd like

Add --quiet or --no-progress-bars option to spfs commit and spfs push commands (and perhaps others with console clearing progress bars) to not output progress bars, or not clear the console/terminal window when using them, or similar. Then people, scripts, and tools have the option of seeing the progress bars or not.

@dcookspi dcookspi added enhancement New feature or request agenda item Items to be brought up at the next dev meeting SPI AOI Area of interest for SPI SPI-0.41 labels May 11, 2024
@dcookspi
Copy link
Collaborator Author

I can see where the Sync args create a ConsoleSyncReporter so adding a flag to that to use the existing SilentSyncReporter might be one way to do this.

Or, we could have the ConsoleSyncReporter take an option to disable the progress bars (I'm assuming they are the cause) but leave the rest of the reporting so there's a summary of the progress ouput.

Or, option 3, we make a new QuieterSyncReporter that doesn't use progress bars but otherwise acts like the existing ConsolerSyncReporter and have a flag in the Sync args to swap that one in as required.

@rydrman
Copy link
Collaborator

rydrman commented May 11, 2024

I would actually love maybe a version of the reporter that can show some minor progress information (rather than sitting silently until it finishes) but also not use progress bars for the reason that you mentioned. I'm curious about the situation here because realistically it should be overwriting data, and also shouldn't be showing progress bars if stdin is not a proper tty... Maybe we are either not using it quite right or something is up in the API.

Is this behaviour being seen in the latest version of spfs?

@dcookspi dcookspi self-assigned this Jul 10, 2024
@dcookspi
Copy link
Collaborator Author

Yes, I can get this behaviour in the latest spk/spfs builds.

I've done some more digging and the problem appears whenever stdout is redirected. You can run any command spk or spfs that involves a sync, which produces progress bars, e.g. spfs push, spfs commit ... and redirect the output to a file or pipe it to another command and the terminal you are in will clear and lose whatever was on screeen before, e.g.

  • spfs push ... > afile
  • spfs push .. | cat
  • somecommandthatcallsspfspush > afile

The progress bar appears to be going to stderr and if just stdout is redirected that messes with the terminal contents. However, if you redirect stderr as well, to a file or pipe, then the progress bars are automatically hidden and the terminal contents remain uncleared.

I started doing some is_tty checks to see what it thought was/wasn't a tty. And I updated the console dependency and the indicatif dependency, and messed with crossterm while doing this, and the problem went away. Backing everything out I traced it to the console dependency update. The newer version doesn't exhibit the problem. So I'll put in a PR for this shortly.

@dcookspi
Copy link
Collaborator Author

dcookspi commented Sep 6, 2024

Even though (we think, it's being tested in production) the screen clearing/redirect problem has gone away, users are asking for an option to hide the progress bars.

So I've reopened this so we can look at adding a --quiet or --no-progress-bars option to the spfs commands to let things that call these commands hide these bars as needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agenda item Items to be brought up at the next dev meeting enhancement New feature or request SPI AOI Area of interest for SPI SPI-0.41
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants