[stdout stream] Added an option to stream outputs#39
Conversation
0f64ffb to
b4f5d8d
Compare
datadog/dogshell/wrap.py
Outdated
There was a problem hiding this comment.
Should we make it True by default ? I think we might want to use this option more often than we won't.
There was a problem hiding this comment.
Yeah, I was wondering as well but so far the behaviour was to have it set to false and I thought maybe some of our clients use dogwrap in some scripts of their owns. So basically I did that for "backward compatibility" but since dogwrap has been released very recently maybe we can put it to True by default. Shall I do that ?
There was a problem hiding this comment.
I don't think 'flushing live' versus 'once the command is completed', can introduce backward compatibility.
|
Did you have a chance to test with Python 2.6 and Python >= 3.3 ? |
b4f5d8d to
a03b226
Compare
a03b226 to
7f9764c
Compare
|
Quick remark about the command line option: |
|
I can't think about any reason why we would rather have the output provided once the command is completed (versus 'real-time'). Should we just drop it and avoid an extra option ? |
|
Yeah, I dont see either to be honest. Maybe we can just enable it by default and add a -b option for (--buffer-outputs) so that we get rid of this letter issue at the same time ! Also maybe adding a --mute or --silent option to prevent the output from being printed may be a good idea even though it's also almost as easy to just redirect the output of dogwrap to /dev/null. What do you guys think ? |
|
I don't really see any use case for the -b option but it doesn't hurt to keep it "just in case". |
|
I like the idea of |
|
👌 |
|
Thanks for doing this—I'm super happy to have realtime output! 👍 Generally, I'm for less flags until we see an actual use case for them. If we can reliably provide realtime output, I don't see any reason not to. But so long as it isn't much extra code, I guess it's nice to leave the I also don't see much case for |
9347987 to
0f204b6
Compare
|
Ok Doug, I just got rid of the silent option. PR should now be ready to be merged. |
- By default, dogwrap now forwards the stdout and stderr of the command it executes to the terminal launching dogwrap. - This behaviour can be prevented using the --buffer_outs option (dogwrap then acts just as it used to do) - I also added some comments in the code. - Finally, I added help messages for the different possible options, as well as an "USAGE" description and and version number (automatically enabled --version option for dogwrap).
0f204b6 to
3491e3e
Compare
[stdout stream] Added an option to stream outputs
it executes to the terminal launching dogwrap.
(dogwrap then acts just as it used to do)
well as an "USAGE" description and and version number (automatically
enabled --version option for dogwrap).