Skip to content

Commit 2b1ec5d

Browse files
committed
Address comments.
1 parent c567876 commit 2b1ec5d

2 files changed

Lines changed: 22 additions & 13 deletions

File tree

sdks/go/container/tools/buffered_logging.go

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package tools
1717

1818
import (
19+
"bytes"
1920
"context"
2021
"log"
2122
"os"
@@ -60,31 +61,39 @@ func (b *BufferedLogger) Write(p []byte) (int, error) {
6061
if b.logger == nil {
6162
return os.Stderr.Write(p)
6263
}
63-
n, err := b.builder.Write(p)
64+
6465
if b.logs == nil {
6566
b.logs = make([]string, 0, initialLogSize)
6667
}
67-
s := b.builder.String()
68+
6869
start := 0
6970
for {
70-
nl := strings.IndexByte(s[start:], '\n')
71+
// Look for the next newline in the incoming byte slice directly
72+
nl := bytes.IndexByte(p[start:], '\n')
7173
if nl == -1 {
7274
break
7375
}
74-
line := s[start : start+nl]
75-
b.logs = append(b.logs, strings.TrimSuffix(line, "\r"))
76+
77+
// Write the segment up to the newline into the builder
78+
b.builder.Write(p[start : start+nl])
79+
80+
// The builder now contains any previous partial line + the current complete segment
81+
b.logs = append(b.logs, strings.TrimSuffix(b.builder.String(), "\r"))
82+
b.builder.Reset()
83+
7684
start += nl + 1
7785
}
78-
if start > 0 {
79-
b.builder.Reset()
80-
if start < len(s) {
81-
b.builder.WriteString(s[start:])
82-
}
86+
87+
// Buffer any remaining bytes that didn't end in a newline
88+
if start < len(p) {
89+
b.builder.Write(p[start:])
8390
}
91+
8492
if b.now().Sub(b.lastFlush) > b.flushInterval {
8593
b.FlushAtDebug(b.periodicFlushContext)
8694
}
87-
return n, err
95+
96+
return len(p), nil
8897
}
8998

9099
// Flush flushes the contents of the buffer to the logging service.

sdks/python/container/piputil.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,14 @@ const pipLogFlushInterval time.Duration = 15 * time.Second
4242
const unrecoverableURL string = "https://beam.apache.org/documentation/sdks/python-unrecoverable-errors/index.html#pip-dependency-resolution-failures"
4343

4444
// executeWithLogger runs the program with os.Stdin, piping stdout and stderr to bufLogger,
45-
// and flushes bufLogger based on the execution result.
45+
// and flushes the logger at ERROR severity on failure or DEBUG severity on success.
4646
func executeWithLogger(ctx context.Context, bufLogger *tools.BufferedLogger, prog string, args ...string) error {
4747
err := execx.ExecuteEnvWithIO(nil, os.Stdin, bufLogger, bufLogger, prog, args...)
4848
return bufLogger.Flush(ctx, err)
4949
}
5050

5151
// executeWithOutput runs the program with os.Stdin, capturing stdout in a byte buffer
52-
// while piping stderr to bufLogger, and flushes bufLogger based on the execution result.
52+
// while piping stderr to bufLogger, and flushes the logger at ERROR severity on failure or DEBUG severity on success.
5353
func executeWithOutput(ctx context.Context, bufLogger *tools.BufferedLogger, prog string, args ...string) ([]byte, error) {
5454
var stdout bytes.Buffer
5555
err := execx.ExecuteEnvWithIO(nil, os.Stdin, &stdout, bufLogger, prog, args...)

0 commit comments

Comments
 (0)