(FACT-865) Split stdout and stderr handling in execution.#900
(FACT-865) Split stdout and stderr handling in execution.#900MikaelSmith merged 1 commit intopuppetlabs:masterfrom
Conversation
peterhuene
force-pushed
the
splitting-stderr
branch
2 times, most recently
from
2e1b759 to
2b31c89
Compare
|
CLA signed by all contributors. |
lib/src/execution/execution.cc
Outdated
There was a problem hiding this comment.
Should we specify which pipe we're closing?
There was a problem hiding this comment.
It should really be "closing child pipes", as both pipes will close when we stop processing. Will fix.
|
I'd argue for always logging stderr if redirect_stderr_to_null is chosen and a severity level of DEBUG or more is chosen. |
There was a problem hiding this comment.
I'll add one for the PS resolver too.
There was a problem hiding this comment.
We also run batch and cmd files on Windows. That's more what I was thinking of, but batch and PS are good.
There was a problem hiding this comment.
Yeah, was just going to add a batch + another test for the PS resolver specifically.
|
First review pass done, nice work. |
peterhuene
force-pushed
the
splitting-stderr
branch
from
2b31c89 to
7795167
Compare
|
I'll see about logging output when redirecting to null with a log level of >= debug. |
peterhuene
force-pushed
the
splitting-stderr
branch
2 times, most recently
from
949d598 to
1cdcbdd
Compare
This implements a new pipe for reading stderr in addition to stdout from child processes. Previously, stderr was optionally redirected to stdout or to the null device if not merging the two streams. This change allows child process execution to treat stdout and stderr as two separate streams, enabling the ability to process stdout while logging stderr messages (e.g. external executable facts). The execute functions now return a tuple of both the stdout and stderr contents. The each_line functions now take an additional function object to call when a line of stderr is read.
peterhuene
force-pushed
the
splitting-stderr
branch
from
1cdcbdd to
8a02176
Compare
|
Okay, all feedback taken into account and this should now (hopefully) pass CI. Ready for one last review. |
|
👍 Looks good to me. |
(FACT-865) Split stdout and stderr handling in execution.
3 participants
This implements a new pipe for reading stderr in addition to stdout from
child processes. Previously, stderr was optionally redirected to
stdout or to the null device if not merging the two streams. This change
allows child process execution to treat stdout and stderr as two
separate streams, enabling the ability to process stdout while logging
stderr messages (e.g. external executable facts).
The execute functions now return a tuple of both the stdout and stderr
contents. The each_line functions now take an additional function
object to call when a line of stderr is read.