whalebrew/whalebrew

Option to run a package without `-it`

hdp617 opened this issue · 3 comments

I think the -it option in docker run is only needed to provide interactive input. When running a command and piping the output e.g. to a tool likejq, interactive input is not needed [0].

I propose adding an option to run a package without -it. For example, we can add a runtime env var like WHALEBREW_USETTY, default to true and we can run a package without interactive input with WHALEBREW_USETTY=false <command>

[0] https://stackoverflow.com/q/65824304

good point! thanks for raising
I believe though this should be detectable from std in/outs
Currently, we use stdin to trigger the --tty argument

This is indeed maybe not enough. Do you have some examples of packages and commands that are suffering the problem?

Thanks for the pointer. I think the same thing can be done for --interactive as well i.e don't set this flag if stdout is not piped to the terminal. Unclear how robust this is. I can take a look!

Do you have some examples of packages and commands that are suffering the problem?

All interactive commands whose output is piped to a file or other tools will suffer the problem. I have an example with the aws CLI in the first comment.

I think the same thing can be done for --interactive as well

I am quite unsure. --interactive is about binding stdin, this allows to pipe whalebrew commands.

image

All interactive commands whose output is piped to a file or other tools will suffer the problem

I actually run most of my clis in whalebrew and have both jq and aws cli pointed in the above post containerized, and didn't have the mentioned problem as illustrated in the snapshot below

image

After some more investigation, I think the fix would be to only open a TTY when both stdin and stdout are TTYs.
This fixes the problem mentioned, but still prevents from being to run the last item in the pipe chain as a whalebrew program.
Say we have less as a whalebrew package, we cannot run aws logs describe-log-groups | less and have the expected behavior.

This is not a regression towards the current behavior. I think it is safe to proceed with such a change, and see how to support the above pattern later on