shunping commented on code in PR #39288:
URL: https://github.com/apache/beam/pull/39288#discussion_r3679344487
##########
sdks/python/container/boot.go:
##########
@@ -580,10 +581,13 @@ func logRuntimeDependencies(ctx context.Context,
bufLogger *tools.BufferedLogger
}
bufLogger.Printf(ctx, "Dependencies in %s:", phase)
args = []string{"-m", "pip", "freeze", "--all"}
- if err := execx.ExecuteEnvWithIO(nil, os.Stdin, bufLogger, bufLogger,
pythonVersion, args...); err != nil {
+
+ var stdout bytes.Buffer
+ if err := execx.ExecuteEnvWithIO(nil, os.Stdin, &stdout, bufLogger,
pythonVersion, args...); err != nil {
Review Comment:
I think the complication here is that the design of `BufferedLogger` is a
hybrid: it acts as both a logger (supporting `Print`) and an `io.Writer`
(supporting `Write`).
For developers who want to use `BufferedLogger` purely as a logger, they
should call `Print`. It behaves just like a standard logger, so there is no
need to worry about manual line splitting or flushing. As described in the
standard Go `log` package:
>Every log message is output on a separate line: if the message being
printed does not end in a newline, the logger will add one.
On the other hand, when `BufferedLogger` is used as an `io.Writer` (e.g.,
capturing `stderr`), it relies on the `Write` method internally. Just like
writing to a standard file buffer, no newline is automatically appended..
Given this distinction, the current ExecuteEnvWithIO feels pretty clean to
me, and I worry that introducing additional APIs might unnecessarily complicate
the interface.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
