Skip to content

Commit 93558ca

Browse files
bduffanybduffany
committed
Feedback
1 parent 8cdc1e4 commit 93558ca

File tree

9 files changed

+509
-139
lines changed

9 files changed

+509
-139
lines changed

‎cli/execute/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ go_library(
1111
"//cli/executions",
1212
"//cli/log",
1313
"//cli/login",
14+
"//cli/markdown",
1415
"//proto:remote_execution_go_proto",
1516
"//server/real_environment",
1617
"//server/remote_cache/digest",

‎cli/execute/execute.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/buildbuddy-io/buildbuddy/cli/executions"
1212
"github.com/buildbuddy-io/buildbuddy/cli/log"
1313
"github.com/buildbuddy-io/buildbuddy/cli/login"
14+
"github.com/buildbuddy-io/buildbuddy/cli/markdown"
1415
"github.com/buildbuddy-io/buildbuddy/server/real_environment"
1516
"github.com/buildbuddy-io/buildbuddy/server/remote_cache/digest"
1617
"github.com/buildbuddy-io/buildbuddy/server/util/bazel_request"
@@ -251,7 +252,8 @@ func execute(cmdArgs []string) error {
251252
return fmt.Errorf("write json output: %w", err)
252253
}
253254
case "markdown":
254-
if _, err := fmt.Fprint(os.Stdout, executions.RenderMarkdownWithDetails(rsp.GetName(), rsp.ExecuteResponse, logs)); err != nil {
255+
markdownWriter := markdown.Writer(os.Stdout, nil)
256+
if err := executions.WriteMarkdownWithDetails(markdownWriter, rsp.GetName(), rsp.ExecuteResponse, logs); err != nil {
255257
return err
256258
}
257259
case "stdio":

‎cli/executions/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ go_library(
1010
"//cli/arg",
1111
"//cli/log",
1212
"//cli/login",
13+
"//cli/markdown",
1314
"//cli/terminal",
1415
"//proto:execution_stats_go_proto",
1516
"//proto:remote_execution_go_proto",

‎cli/executions/executions.go‎

Lines changed: 73 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/buildbuddy-io/buildbuddy/cli/arg"
1616
"github.com/buildbuddy-io/buildbuddy/cli/log"
1717
"github.com/buildbuddy-io/buildbuddy/cli/login"
18+
"github.com/buildbuddy-io/buildbuddy/cli/markdown"
1819
"github.com/buildbuddy-io/buildbuddy/cli/terminal"
1920
"github.com/buildbuddy-io/buildbuddy/server/util/grpc_client"
2021
"github.com/buildbuddy-io/buildbuddy/server/util/rexec"
@@ -118,8 +119,11 @@ func handleGet(args []string) (int, error) {
118119
return 0, err
119120
}
120121

121-
_, err = fmt.Fprint(os.Stdout, RenderMarkdown(executionID, executeResponse))
122-
return 0, err
122+
markdownWriter := markdown.Writer(os.Stdout, nil)
123+
if err := WriteMarkdown(markdownWriter, executionID, executeResponse); err != nil {
124+
return 0, err
125+
}
126+
return 0, nil
123127
}
124128

125129
func fetchExecuteResponse(ctx context.Context, target, executionID string) (*repb.ExecuteResponse, error) {
@@ -133,9 +137,9 @@ func fetchExecuteResponse(ctx context.Context, target, executionID string) (*rep
133137
return rexec.GetCachedExecuteResponse(ctx, acClient, executionID)
134138
}
135139

136-
// RenderMarkdown renders a human-friendly markdown summary of an ExecuteResponse.
137-
func RenderMarkdown(executionID string, executeResponse *repb.ExecuteResponse) string {
138-
return RenderMarkdownWithDetails(executionID, executeResponse, nil)
140+
// WriteMarkdown writes a human-friendly markdown summary of an ExecuteResponse.
141+
func WriteMarkdown(w io.Writer, executionID string, executeResponse *repb.ExecuteResponse) error {
142+
return WriteMarkdownWithDetails(w, executionID, executeResponse, nil)
139143
}
140144

141145
type jsonOutput struct {
@@ -193,29 +197,27 @@ func WriteJSONOutput(w io.Writer, executionID string, executeResponse *repb.Exec
193197
return nil
194198
}
195199

196-
// RenderMarkdownWithDetails renders a human-friendly markdown summary of an
200+
// WriteMarkdownWithDetails writes a human-friendly markdown summary of an
197201
// ExecuteResponse, optionally including fetched stdout/stderr details.
198-
func RenderMarkdownWithDetails(executionID string, executeResponse *repb.ExecuteResponse, details *rexec.ExecutionLogs) string {
199-
var b strings.Builder
202+
func WriteMarkdownWithDetails(w io.Writer, executionID string, executeResponse *repb.ExecuteResponse, details *rexec.ExecutionLogs) error {
200203
statusCode := codes.Code(executeResponse.GetStatus().GetCode())
201204
statusColor := colorForStatus(statusCode)
202205
var stdout, stderr []byte
203206
if details != nil {
204207
stdout = details.Stdout
205208
stderr = details.Stderr
206209
}
207-
208-
b.WriteString(colorHeading("# Execution details\n"))
209-
b.WriteString(fmt.Sprintf("- Execution ID: `%s`\n", executionID))
210-
b.WriteString("- Stage: Completed\n")
211-
b.WriteString(fmt.Sprintf("- RPC status: %s%s (%d)%s", statusColor, statusCode.String(), statusCode, terminal.Esc()))
210+
io.WriteString(w, "# Execution details\n")
211+
fmt.Fprintf(w, "- Execution ID: `%s`\n", executionID)
212+
io.WriteString(w, "- Stage: Completed\n")
213+
fmt.Fprintf(w, "- RPC status: %s%s (%d)%s", statusColor, statusCode.String(), statusCode, terminal.Esc())
212214
if msg := executeResponse.GetStatus().GetMessage(); msg != "" {
213-
b.WriteString(fmt.Sprintf(": %s", msg))
215+
fmt.Fprintf(w, ": %s", msg)
214216
}
215-
b.WriteString("\n")
216-
b.WriteString(fmt.Sprintf("- Served from cache: %s\n", yesNo(executeResponse.GetCachedResult())))
217+
io.WriteString(w, "\n")
218+
fmt.Fprintf(w, "- Served from cache: %s\n", yesNo(executeResponse.GetCachedResult()))
217219
if msg := executeResponse.GetMessage(); msg != "" {
218-
b.WriteString(fmt.Sprintf("- Message: %s\n", msg))
220+
fmt.Fprintf(w, "- Message: %s\n", msg)
219221
}
220222

221223
result := executeResponse.GetResult()
@@ -227,23 +229,23 @@ func RenderMarkdownWithDetails(executionID string, executeResponse *repb.Execute
227229
stderr = result.GetStderrRaw()
228230
}
229231

230-
b.WriteString("\n")
231-
b.WriteString(colorHeading("# Result details\n"))
232-
b.WriteString(fmt.Sprintf("- Exit code: %d\n", result.GetExitCode()))
233-
b.WriteString(fmt.Sprintf("- Outputs: %d files, %d directories, %d symlinks\n", len(result.GetOutputFiles()), len(result.GetOutputDirectories()), len(result.GetOutputSymlinks())))
234-
b.WriteString(fmt.Sprintf("- Stdout: %s\n", outputSummary(len(result.GetStdoutRaw()), result.GetStdoutDigest())))
235-
b.WriteString(fmt.Sprintf("- Stderr: %s\n", outputSummary(len(result.GetStderrRaw()), result.GetStderrDigest())))
232+
io.WriteString(w, "\n")
233+
io.WriteString(w, "# Result details\n")
234+
fmt.Fprintf(w, "- Exit code: %d\n", result.GetExitCode())
235+
fmt.Fprintf(w, "- Outputs: %d files, %d directories, %d symlinks\n", len(result.GetOutputFiles()), len(result.GetOutputDirectories()), len(result.GetOutputSymlinks()))
236+
fmt.Fprintf(w, "- Stdout: %s\n", outputSummary(len(result.GetStdoutRaw()), result.GetStdoutDigest()))
237+
fmt.Fprintf(w, "- Stderr: %s\n", outputSummary(len(result.GetStderrRaw()), result.GetStderrDigest()))
236238

237239
if len(executeResponse.GetServerLogs()) > 0 {
238-
b.WriteString("- Server logs:\n")
240+
io.WriteString(w, "- Server logs:\n")
239241
logNames := make([]string, 0, len(executeResponse.GetServerLogs()))
240242
for name := range executeResponse.GetServerLogs() {
241243
logNames = append(logNames, name)
242244
}
243245
slices.Sort(logNames)
244246
for _, name := range logNames {
245247
logFile := executeResponse.GetServerLogs()[name]
246-
b.WriteString(fmt.Sprintf(" - %s: %s\n", name, digestString(logFile.GetDigest())))
248+
fmt.Fprintf(w, " - %s: %s\n", name, digestString(logFile.GetDigest()))
247249
}
248250
}
249251

@@ -255,35 +257,32 @@ func RenderMarkdownWithDetails(executionID string, executeResponse *repb.Execute
255257
workerQueuedTS = formatTimestamp(aux.GetWorkerQueuedTimestamp())
256258
}
257259

258-
b.WriteString("\n")
259-
b.WriteString(colorHeading("# Execution metadata\n"))
260-
b.WriteString(fmt.Sprintf("- Worker: %s\n", emptyIfUnset(md.GetWorker())))
261-
b.WriteString(fmt.Sprintf("- Executor ID: %s\n", emptyIfUnset(md.GetExecutorId())))
262-
b.WriteString(fmt.Sprintf("- Queued: %s\n", formatTimestamp(md.GetQueuedTimestamp())))
263-
b.WriteString(fmt.Sprintf("- Worker queued: %s\n", workerQueuedTS))
264-
b.WriteString(fmt.Sprintf("- Worker started: %s\n", formatTimestamp(md.GetWorkerStartTimestamp())))
265-
b.WriteString(fmt.Sprintf("- Worker completed: %s\n", formatTimestamp(md.GetWorkerCompletedTimestamp())))
266-
b.WriteString(fmt.Sprintf("- Input fetch duration: %s\n", formatDuration(md.GetInputFetchStartTimestamp(), md.GetInputFetchCompletedTimestamp())))
267-
b.WriteString(fmt.Sprintf("- Execution duration: %s\n", formatDuration(md.GetExecutionStartTimestamp(), md.GetExecutionCompletedTimestamp())))
268-
b.WriteString(fmt.Sprintf("- Output upload duration: %s\n", formatDuration(md.GetOutputUploadStartTimestamp(), md.GetOutputUploadCompletedTimestamp())))
269-
b.WriteString(fmt.Sprintf("- Worker total duration: %s\n", formatDuration(md.GetWorkerStartTimestamp(), md.GetWorkerCompletedTimestamp())))
260+
io.WriteString(w, "\n")
261+
io.WriteString(w, "# Execution metadata\n")
262+
fmt.Fprintf(w, "- Worker: %s\n", emptyIfUnset(md.GetWorker()))
263+
fmt.Fprintf(w, "- Executor ID: %s\n", emptyIfUnset(md.GetExecutorId()))
264+
fmt.Fprintf(w, "- Queued: %s\n", formatTimestamp(md.GetQueuedTimestamp()))
265+
fmt.Fprintf(w, "- Worker queued: %s\n", workerQueuedTS)
266+
fmt.Fprintf(w, "- Worker started: %s\n", formatTimestamp(md.GetWorkerStartTimestamp()))
267+
fmt.Fprintf(w, "- Worker completed: %s\n", formatTimestamp(md.GetWorkerCompletedTimestamp()))
268+
fmt.Fprintf(w, "- Input fetch duration: %s\n", formatDuration(md.GetInputFetchStartTimestamp(), md.GetInputFetchCompletedTimestamp()))
269+
fmt.Fprintf(w, "- Execution duration: %s\n", formatDuration(md.GetExecutionStartTimestamp(), md.GetExecutionCompletedTimestamp()))
270+
fmt.Fprintf(w, "- Output upload duration: %s\n", formatDuration(md.GetOutputUploadStartTimestamp(), md.GetOutputUploadCompletedTimestamp()))
271+
fmt.Fprintf(w, "- Worker total duration: %s\n", formatDuration(md.GetWorkerStartTimestamp(), md.GetWorkerCompletedTimestamp()))
270272
}
271273
}
272274

273-
b.WriteString("\n")
274-
b.WriteString(colorHeading("# Execution stdout\n"))
275-
b.WriteString(renderTextSection(stdout))
276-
b.WriteString("\n")
277-
b.WriteString(colorHeading("# Execution stderr\n"))
278-
b.WriteString(renderTextSection(stderr))
279-
b.WriteString("\n")
280-
b.WriteString(colorHeading("# Execution server logs\n"))
281-
b.WriteString(renderServerLogsSection(executeResponse, details))
282-
return b.String()
283-
}
275+
io.WriteString(w, "\n")
276+
io.WriteString(w, "# Execution stdout\n")
277+
writeTextSection(w, stdout)
278+
io.WriteString(w, "\n")
279+
io.WriteString(w, "# Execution stderr\n")
280+
writeTextSection(w, stderr)
281+
io.WriteString(w, "\n")
282+
io.WriteString(w, "# Execution server logs\n")
283+
writeServerLogsSection(w, executeResponse, details)
284284

285-
func colorHeading(s string) string {
286-
return terminal.Esc(1, 36) + s + terminal.Esc()
285+
return writerErr(w)
287286
}
288287

289288
func colorForStatus(code codes.Code) string {
@@ -342,42 +341,44 @@ func formatDuration(start, end *timestamppb.Timestamp) string {
342341
return d.String()
343342
}
344343

345-
func renderTextSection(data []byte) string {
344+
func writeTextSection(w io.Writer, data []byte) {
346345
if len(data) == 0 {
347-
return "(Empty)\n"
346+
io.WriteString(w, "(Empty)\n")
347+
return
348348
}
349349
if !utf8.Valid(data) {
350-
return fmt.Sprintf("<binary: %d bytes>\n", len(data))
350+
fmt.Fprintf(w, "<binary: %d bytes>\n", len(data))
351+
return
351352
}
352-
out := "```\n" + string(data)
353+
io.WriteString(w, "```\n")
354+
w.Write(data)
353355
if data[len(data)-1] != '\n' {
354-
out += "\n"
356+
io.WriteString(w, "\n")
355357
}
356-
out += "```\n"
357-
return out
358+
io.WriteString(w, "```\n")
358359
}
359360

360-
func renderServerLogsSection(executeResponse *repb.ExecuteResponse, details *rexec.ExecutionLogs) string {
361+
func writeServerLogsSection(w io.Writer, executeResponse *repb.ExecuteResponse, details *rexec.ExecutionLogs) {
361362
if details != nil && len(details.ServerLogs) > 0 {
362363
logNames := make([]string, 0, len(details.ServerLogs))
363364
for name := range details.ServerLogs {
364365
logNames = append(logNames, name)
365366
}
366367
slices.Sort(logNames)
367368

368-
var b strings.Builder
369369
for i, name := range logNames {
370370
if i > 0 {
371-
b.WriteString("\n")
371+
io.WriteString(w, "\n")
372372
}
373-
b.WriteString(colorHeading(fmt.Sprintf("## %s\n", name)))
374-
b.WriteString(renderTextSection(details.ServerLogs[name]))
373+
fmt.Fprintf(w, "## %s\n", name)
374+
writeTextSection(w, details.ServerLogs[name])
375375
}
376-
return b.String()
376+
return
377377
}
378378

379379
if len(executeResponse.GetServerLogs()) == 0 {
380-
return "(Empty)\n"
380+
io.WriteString(w, "(Empty)\n")
381+
return
381382
}
382383

383384
// Server logs exist but were not fetched in this code path.
@@ -386,10 +387,15 @@ func renderServerLogsSection(executeResponse *repb.ExecuteResponse, details *rex
386387
logNames = append(logNames, name)
387388
}
388389
slices.Sort(logNames)
389-
var b strings.Builder
390390
for _, name := range logNames {
391391
logFile := executeResponse.GetServerLogs()[name]
392-
b.WriteString(fmt.Sprintf("- %s: %s\n", name, digestString(logFile.GetDigest())))
392+
fmt.Fprintf(w, "- %s: %s\n", name, digestString(logFile.GetDigest()))
393393
}
394-
return b.String()
394+
}
395+
396+
func writerErr(w io.Writer) error {
397+
if w, ok := w.(interface{ Err() error }); ok {
398+
return w.Err()
399+
}
400+
return nil
395401
}

‎cli/markdown/BUILD‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
2+
3+
package(default_visibility = ["//cli:__subpackages__"])
4+
5+
go_library(
6+
name = "markdown",
7+
srcs = ["markdown.go"],
8+
importpath = "github.com/buildbuddy-io/buildbuddy/cli/markdown",
9+
deps = ["//cli/terminal"],
10+
)
11+
12+
go_test(
13+
name = "markdown_test",
14+
srcs = ["markdown_test.go"],
15+
embed = [":markdown"],
16+
)

0 commit comments

Comments
 (0)