Closed utkarshgupta137 closed 6 months ago
I agree the progress bar can be more configurable than current hard-codings, I think at least we want these options:
We may even want to make the entire PROGRESS_STYLE configurable as there are may options that Indicatif suppports.
I like using CLI arguments for above options. Would be happy to review a PR @utkarshgupta137
Thanks!
I am thinking we can add these 2 command line options easily:
--progress-bar-disable-steady-tick
--progress-bar-disable-spinner
@utkarshgupta137 please let me know if you would like to raise a PR for above, if not I am happy to make this change also.
thanks!
I would personally prefer an environment variable for this, otherwise it would crowd out the command line too much. I feel like this is something that you either always want on or always want off & not toggle it on a per invocation basis. Can have both too.
FYI, there is precedent for this kind of stuff being toggled on or off using environment variables, for example NO_COLOR
or CI
environment variables.
I pushed a commit here: https://github.com/aaronriekenberg/rust-parallel/commit/24b8257663ad3541ac21ab282342fa4552bcb07c
This disables the steady tick and spinner if stderr is not a terminal, using the builtin std::io::stderr().is_terminal()
@utkarshgupta137 can you please test if this behaves well in your airflow job example?
thanks!
No, doesn't seem to be working. This could partly be because I use ssh command with a pseudo-terminal enabled, so that tasks get killed when I kill them from airflow.
Thanks for testing @utkarshgupta137
One question to understand - is this the way you are running your jobs?
airflow -> ssh -t (force pseudo-terminal) -> rust-parallel (execute commands in parallel)
Thanks for testing @utkarshgupta137
One question to understand - is this the way you are running your jobs?
airflow -> ssh -t (force pseudo-terminal) -> rust-parallel (execute commands in parallel)
Yeah, it uses get_pty arg for paramiko.
Learned that tracing subscriber (which rust-parallel uses for logging) uses NO_COLOR
environment variable to decide if terminal supports ANSI. This is a useful thing on its own, if NO_COLOR=1
environment variable is set, rust-parallel logging will not use ANSI color sequences.
References:
Pushed a commit to make the progress bar check if NO_COLOR environment variable is set to non-empty value. If yes then we decide the terminal does not support ANSI, so disable steady tick and spinner in progress bar: https://github.com/aaronriekenberg/rust-parallel/commit/10d60198303cc10abd372cf03f823107d1306078
Example usage:
$ export NO_COLOR=1
$ rust-parallel -d all -p sleep ::: 1 2 3
OR
$ NO_COLOR=1 rust-parallel -d all -p sleep ::: 1 2 3
@utkarshgupta137 can you please test this? Thanks!
In v1.17.0 added PROGRESS_STYLE
environment variable to allow for user-configurable progress styles.
See progress bar manual section for more details.
@utkarshgupta137 I think PROGRESS_STYLE=simple
should work well for your jobs - this will disable spinner and steady tick.
Thanks!
Thank you, it is indeed working.
We use rust-parallel in an airflow job. Because of the way airflow saves the logs of the jobs, it creates a new line every time the spinner is updated, as it doesn't support ANSI I think. This results in a lot of spam, since this crate has a steady tick of 100ms, we get 2.1M useless log lines, which not only makes the logs useless, it also increases the log storage costs. I've a fork where I've removed both of these, but I would like to raise a PR with an option to disable this at runtime either via a CLI arg or environment variable (such as TERM or NO_COLOR). What kind of an interface should we have for this.