Make the ros2cli output always line buffered

[ivanpauno]

Fixes #595.

This forces all calls to print() to also flush the output buffer, the result is similar to having line buffering.

Alternative: We could also do something like what it's suggested here https://stackoverflow.com/questions/107705/disable-output-buffering.

Another alternative: I don't think we need unbuffered output for all commands though, e.g. it's not needed for ros2 topic list.
The ones that need unbuffered (or line buffered) output are the ones that are periodically reporting progress, e.g. ros2 topic pub, ros2 topic echo, etc.
Some of that output could arguably be going to stderr, which in python is always line buffered (even when non interactive), e.g. the output of ros2 topic pub.
The output of ros2 topic echo should probably go to stdout though, and if we don't want global line buffering we would need to pass the flush argument manually.

See also:

• https://docs.python.org/3/library/sys.html#sys.stderr
• https://stackoverflow.com/questions/230751/how-can-i-flush-the-output-of-the-print-function
• https://stackoverflow.com/questions/881696/unbuffered-stdout-in-python-as-in-python-u-from-within-the-program

[hidmic]

I like the approach, but it'd be nice to have a way to toggle that flag (e.g. an envvar, a CLI argument, take your pick).

[ivanpauno]

I like the approach, but it'd be nice to have a way to toggle that flag (e.g. an envvar, a CLI argument, take your pick).

I can add that, but I'm not sure why someone would want to use it.
do you have an use case in mind?

[clalancette]

I like the approach. The only real reason to allow disabling of flushing is for performance, but I actually don't think that is super important here.

I think we should run full CI on this, just to make sure something above this package doesn't break.

Finally, when we do merge this I'll request a note on https://github.com/ros2/ros2_documentation/blob/rolling/source/Releases/Release-Humble-Hawksbill.rst , since it is a change in behavior.

[wjwwood]

After talking offline about it, the use case I think is steaming relatively high bandwidth data from echo into a file for processing later. In that case performance could be an issue. So I'm with @hidmic and there should be a way to "undo" our always-unbuffered magic.

[ivanpauno]

I have made two changes:

• Added an argument to disable line buffering.
• Use TextIoWrapper.reconfigure() instead of patching print().
  The advantage is that only affects stdout, and not print() to other files.
  stdout is documented to be a FileObject as returned by open(), and open() docs say that they return a TextIoWrapper instance for files in text mode (which stdout always is).
  So I would assume that means that stdout is always a TextIoWrapper instance with a reconfigure() method (except if someone patched sys.stdout with a StringIO stream before, which cannot happen here).

[ivanpauno]

Use TextIoWrapper.reconfigure() instead of patching print().

I noticed while writing this that that method was added in python 3.7, and we still support python 3.6.
So, I will add a fallback mechanism to support line buffering (or use the previous print() patch).

[wjwwood]

lgtm

[ivanpauno]

CI:

• Linux
• Linux-aarch64
• macOS
• Windows

[ivanpauno]

Windows failure is unrelated

[clalancette]

Two nits inline, but I'm going to approve anyway.

[jacobperron]

lgtm with Chris's comments addressed.

[hidmic]

@ivanpauno FYI if the decorator monitors the file keyword argument of print, you can limit its action to sys.stdout.

[ivanpauno]

true, I didn't feel that was really needed though.
do you think we need a patch for this?

[hidmic]

Nope, it's unlikely anyone will ever hit it before Python 3.6 support is gone.

[fujitatomoya]

This breaks the CI for ros2param (see https://ci.ros2.org/job/ci_linux/15050/testReport/junit/ros2param.ros2param.test/test_verb_dump/test_verb_dump/), we need to revert this now.