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` and the apis of
`BufferedLogger` 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]
