Skip to content

Commit fba0411

Browse files
committed
fix: address PR review comments for piping-composability
- Return errors from runBatchStart instead of silently swallowing them - Remove unused Piped field from StartOptions - Add scanner.Err() check after stdin scan loop in piping.go
1 parent 03b8f8c commit fba0411

2 files changed

Lines changed: 13 additions & 6 deletions

File tree

pkg/cmd/start/start.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package start
44
import (
55
"fmt"
66
"net/url"
7+
"os"
78
"path/filepath"
89
"strings"
910
"time"
@@ -19,6 +20,7 @@ import (
1920
"github.com/brevdev/brev-cli/pkg/store"
2021
"github.com/brevdev/brev-cli/pkg/terminal"
2122
"github.com/brevdev/brev-cli/pkg/util"
23+
"github.com/hashicorp/go-multierror"
2224
"github.com/spf13/cobra"
2325
)
2426

@@ -114,7 +116,6 @@ type StartOptions struct {
114116
WorkspaceClass string
115117
Detached bool
116118
InstanceType string
117-
Piped bool // true when stdout is piped to another command
118119
}
119120

120121
func runStartWorkspace(t *terminal.Terminal, options StartOptions, startStore StartStore) error {
@@ -679,6 +680,7 @@ func pollUntil(t *terminal.Terminal, wsid string, state string, startStore Start
679680
// runBatchStart handles starting multiple instances when stdin is piped
680681
func runBatchStart(t *terminal.Terminal, names []string, org, setupScript, setupRepo, setupPath, cpu, gpu string, piped bool, startStore StartStore) error {
681682
var startedNames []string
683+
var errs error
682684
for _, instanceName := range names {
683685
err := runStartWorkspace(t, StartOptions{
684686
RepoOrPathOrNameOrID: instanceName,
@@ -690,12 +692,10 @@ func runBatchStart(t *terminal.Terminal, names []string, org, setupScript, setup
690692
WorkspaceClass: cpu,
691693
Detached: true, // Always detached when piping multiple
692694
InstanceType: gpu,
693-
Piped: piped,
694695
}, startStore)
695696
if err != nil {
696-
if !piped {
697-
t.Vprintf("Error starting %s: %s\n", instanceName, err.Error())
698-
}
697+
fmt.Fprintf(os.Stderr, "Error starting %s: %s\n", instanceName, err.Error())
698+
errs = multierror.Append(errs, err)
699699
} else {
700700
startedNames = append(startedNames, instanceName)
701701
}
@@ -706,6 +706,9 @@ func runBatchStart(t *terminal.Terminal, names []string, org, setupScript, setup
706706
fmt.Println(n)
707707
}
708708
}
709+
if errs != nil {
710+
return breverrors.WrapAndTrace(errs)
711+
}
709712
return nil
710713
}
711714

@@ -726,7 +729,6 @@ func runSingleStart(t *terminal.Terminal, names []string, name, org, setupScript
726729
WorkspaceClass: cpu,
727730
Detached: detached,
728731
InstanceType: gpu,
729-
Piped: piped,
730732
}, startStore)
731733
if err != nil {
732734
if strings.Contains(err.Error(), "duplicate instance with name") {

pkg/cmd/util/piping.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package util
22

33
import (
44
"bufio"
5+
"fmt"
56
"os"
67
"strings"
78

@@ -47,6 +48,10 @@ func GetInstanceNamesWithPipeInfo(args []string) ([]string, bool) {
4748
names = append(names, name)
4849
}
4950
}
51+
if err := scanner.Err(); err != nil {
52+
// Log but don't fail - partial input is still useful
53+
fmt.Fprintf(os.Stderr, "warning: error reading stdin: %v\n", err)
54+
}
5055
}
5156

5257
return names, stdinPiped

0 commit comments

Comments
 (0)