Skip to content

Commit b2105c9

Browse files
mrnuggetmrnugget
authored
Extract code from batches package into smaller sub-packages (#511)
* Create executor, git, service packages * Rename Opts in executor * Rename ServiceOpts * Move LogManager to log package * workspace pkg, unexport types and rename WorkspaceCreator to Creator * Remove unused file
1 parent 6dfc92f commit b2105c9

40 files changed

+855
-721
lines changed

‎cmd/src/batch_apply.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"flag"
77
"fmt"
88

9-
"github.com/sourcegraph/src-cli/internal/batches"
9+
"github.com/sourcegraph/src-cli/internal/batches/service"
1010
"github.com/sourcegraph/src-cli/internal/output"
1111
)
1212

@@ -30,7 +30,7 @@ Examples:
3030
flagSet := flag.NewFlagSet("apply", flag.ExitOnError)
3131
flags := newBatchApplyFlags(flagSet, batchDefaultCacheDir(), batchDefaultTempDirPrefix())
3232

33-
doApply := func(ctx context.Context, out *output.Output, svc *batches.Service, flags *batchApplyFlags) error {
33+
doApply := func(ctx context.Context, out *output.Output, svc *service.Service, flags *batchApplyFlags) error {
3434
id, _, err := batchExecute(ctx, out, svc, flags)
3535
if err != nil {
3636
return err
@@ -67,7 +67,7 @@ Examples:
6767
ctx, cancel := contextCancelOnInterrupt(context.Background())
6868
defer cancel()
6969

70-
svc := batches.NewService(&batches.ServiceOpts{
70+
svc := service.New(&service.Opts{
7171
AllowUnsupported: flags.allowUnsupported,
7272
AllowIgnored: flags.allowIgnored,
7373
Client: cfg.apiClient(flags.api, flagSet.Output()),

‎cmd/src/batch_common.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ import (
1919
"github.com/sourcegraph/go-diff/diff"
2020
"github.com/sourcegraph/src-cli/internal/api"
2121
"github.com/sourcegraph/src-cli/internal/batches"
22+
"github.com/sourcegraph/src-cli/internal/batches/executor"
2223
"github.com/sourcegraph/src-cli/internal/batches/graphql"
24+
"github.com/sourcegraph/src-cli/internal/batches/service"
2325
"github.com/sourcegraph/src-cli/internal/output"
2426
)
2527

@@ -189,7 +191,7 @@ func batchOpenFileFlag(flag *string) (io.ReadCloser, error) {
189191
// batchExecute performs all the steps required to upload the campaign spec
190192
// to Sourcegraph, including execution as needed. The return values are the
191193
// spec ID, spec URL, and error.
192-
func batchExecute(ctx context.Context, out *output.Output, svc *batches.Service, flags *batchApplyFlags) (graphql.BatchSpecID, string, error) {
194+
func batchExecute(ctx context.Context, out *output.Output, svc *service.Service, flags *batchApplyFlags) (graphql.BatchSpecID, string, error) {
193195
if err := checkExecutable("git", "version"); err != nil {
194196
return "", "", err
195197
}
@@ -275,13 +277,12 @@ func batchExecute(ctx context.Context, out *output.Output, svc *batches.Service,
275277
task.Archive = fetcher.Checkout(task.Repository, task.ArchivePathToFetch())
276278
}
277279

278-
opts := batches.ExecutorOpts{
279-
Cache: svc.NewExecutionCache(flags.cacheDir),
280+
opts := executor.Opts{
281+
TempDir: flags.tempDir,
280282
Creator: workspaceCreator,
281283
ClearCache: flags.clearCache,
282284
KeepLogs: flags.keepLogs,
283285
Timeout: flags.timeout,
284-
TempDir: flags.tempDir,
285286
Parallelism: flags.parallelism,
286287
}
287288

@@ -354,7 +355,7 @@ func batchExecute(ctx context.Context, out *output.Output, svc *batches.Service,
354355
// batchParseSpec parses and validates the given batch spec. If the spec has
355356
// validation errors, the errors are output in a human readable form and an
356357
// exitCodeError is returned.
357-
func batchParseSpec(out *output.Output, svc *batches.Service, input io.ReadCloser) (*batches.BatchSpec, string, error) {
358+
func batchParseSpec(out *output.Output, svc *service.Service, input io.ReadCloser) (*batches.BatchSpec, string, error) {
358359
spec, raw, err := svc.ParseBatchSpec(input)
359360
if err != nil {
360361
if merr, ok := err.(*multierror.Error); ok {
@@ -400,7 +401,7 @@ func printExecutionError(out *output.Output, err error) {
400401
}
401402

402403
for _, e := range errs {
403-
if taskErr, ok := e.(batches.TaskExecutionErr); ok {
404+
if taskErr, ok := e.(executor.TaskExecutionErr); ok {
404405
block.Write(formatTaskExecutionErr(taskErr))
405406
} else {
406407
if err == context.Canceled {
@@ -455,7 +456,7 @@ func flattenErrs(err error) (result []error) {
455456
return result
456457
}
457458

458-
func formatTaskExecutionErr(err batches.TaskExecutionErr) string {
459+
func formatTaskExecutionErr(err executor.TaskExecutionErr) string {
459460
if ee, ok := errors.Cause(err).(*exec.ExitError); ok && ee.String() == "signal: killed" {
460461
return fmt.Sprintf(
461462
"%s%s%s: killed by interrupt signal",

‎cmd/src/batch_preview.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"flag"
77
"fmt"
88

9-
"github.com/sourcegraph/src-cli/internal/batches"
9+
"github.com/sourcegraph/src-cli/internal/batches/service"
1010
"github.com/sourcegraph/src-cli/internal/output"
1111
)
1212

@@ -42,7 +42,7 @@ Examples:
4242
ctx, cancel := contextCancelOnInterrupt(context.Background())
4343
defer cancel()
4444

45-
svc := batches.NewService(&batches.ServiceOpts{
45+
svc := service.New(&service.Opts{
4646
AllowUnsupported: flags.allowUnsupported,
4747
AllowIgnored: flags.allowIgnored,
4848
Client: cfg.apiClient(flags.api, flagSet.Output()),

‎cmd/src/batch_progress_printer.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"strings"
77

88
"github.com/sourcegraph/go-diff/diff"
9-
"github.com/sourcegraph/src-cli/internal/batches"
9+
"github.com/sourcegraph/src-cli/internal/batches/executor"
1010
"github.com/sourcegraph/src-cli/internal/output"
1111
"golang.org/x/sync/semaphore"
1212
)
@@ -22,7 +22,7 @@ func newBatchProgressPrinter(out *output.Output, verbose bool, numParallelism in
2222
numParallelism: numParallelism,
2323

2424
completedTasks: map[string]bool{},
25-
runningTasks: map[string]*batches.TaskStatus{},
25+
runningTasks: map[string]*executor.TaskStatus{},
2626

2727
repoStatusBar: map[string]int{},
2828
statusBarRepo: map[int]string{},
@@ -46,13 +46,13 @@ type batchProgressPrinter struct {
4646
numParallelism int
4747

4848
completedTasks map[string]bool
49-
runningTasks map[string]*batches.TaskStatus
49+
runningTasks map[string]*executor.TaskStatus
5050

5151
repoStatusBar map[string]int
5252
statusBarRepo map[int]string
5353
}
5454

55-
func (p *batchProgressPrinter) initProgressBar(statuses []*batches.TaskStatus) int {
55+
func (p *batchProgressPrinter) initProgressBar(statuses []*executor.TaskStatus) int {
5656
numStatusBars := p.numParallelism
5757
if len(statuses) < numStatusBars {
5858
numStatusBars = len(statuses)
@@ -93,7 +93,7 @@ func (p *batchProgressPrinter) updateProgressBar(completed, errored, total int)
9393
p.progress.SetLabelAndRecalc(0, label)
9494
}
9595

96-
func (p *batchProgressPrinter) PrintStatuses(statuses []*batches.TaskStatus) {
96+
func (p *batchProgressPrinter) PrintStatuses(statuses []*executor.TaskStatus) {
9797
if len(statuses) == 0 {
9898
return
9999
}
@@ -109,8 +109,8 @@ func (p *batchProgressPrinter) PrintStatuses(statuses []*batches.TaskStatus) {
109109
p.numStatusBars = p.initProgressBar(statuses)
110110
}
111111

112-
newlyCompleted := []*batches.TaskStatus{}
113-
currentlyRunning := []*batches.TaskStatus{}
112+
newlyCompleted := []*executor.TaskStatus{}
113+
currentlyRunning := []*executor.TaskStatus{}
114114
errored := 0
115115

116116
for _, ts := range statuses {
@@ -145,7 +145,7 @@ func (p *batchProgressPrinter) PrintStatuses(statuses []*batches.TaskStatus) {
145145

146146
p.updateProgressBar(len(p.completedTasks), errored, len(statuses))
147147

148-
newlyStarted := map[string]*batches.TaskStatus{}
148+
newlyStarted := map[string]*executor.TaskStatus{}
149149
statusBarIndex := 0
150150
for _, ts := range currentlyRunning {
151151
if _, ok := p.runningTasks[ts.DisplayName()]; ok {
@@ -252,7 +252,7 @@ type statusTexter interface {
252252
StatusText() string
253253
}
254254

255-
func taskStatusBarText(ts *batches.TaskStatus) (string, error) {
255+
func taskStatusBarText(ts *executor.TaskStatus) (string, error) {
256256
var statusText string
257257

258258
if ts.IsCompleted() {

‎cmd/src/batch_progress_printer_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
"github.com/google/go-cmp/cmp"
1010
"github.com/sourcegraph/src-cli/internal/batches"
11+
"github.com/sourcegraph/src-cli/internal/batches/executor"
1112
"github.com/sourcegraph/src-cli/internal/output"
1213
)
1314

@@ -52,7 +53,7 @@ func TestBatchProgressPrinterIntegration(t *testing.T) {
5253
})
5354

5455
now := time.Now()
55-
statuses := []*batches.TaskStatus{
56+
statuses := []*executor.TaskStatus{
5657
{
5758
RepoName: "github.com/sourcegraph/sourcegraph",
5859
StartedAt: now,
@@ -89,7 +90,7 @@ func TestBatchProgressPrinterIntegration(t *testing.T) {
8990
}
9091

9192
// Now mark the last task as completed
92-
statuses[len(statuses)-1] = &batches.TaskStatus{
93+
statuses[len(statuses)-1] = &executor.TaskStatus{
9394
RepoName: "github.com/sourcegraph/automation-testing",
9495
StartedAt: now.Add(time.Duration(-5) * time.Second),
9596
FinishedAt: now.Add(time.Duration(5) * time.Second),

‎cmd/src/batch_repositories.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import (
77

88
"github.com/pkg/errors"
99
"github.com/sourcegraph/src-cli/internal/api"
10-
"github.com/sourcegraph/src-cli/internal/batches"
1110
"github.com/sourcegraph/src-cli/internal/batches/graphql"
11+
"github.com/sourcegraph/src-cli/internal/batches/service"
1212
"github.com/sourcegraph/src-cli/internal/output"
1313
)
1414

@@ -48,7 +48,7 @@ Examples:
4848
ctx := context.Background()
4949
client := cfg.apiClient(apiFlags, flagSet.Output())
5050

51-
svc := batches.NewService(&batches.ServiceOpts{Client: client})
51+
svc := service.New(&service.Opts{Client: client})
5252

5353
if err := svc.DetermineFeatureFlags(ctx); err != nil {
5454
return err

‎cmd/src/batch_validate.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import (
55
"flag"
66
"fmt"
77

8-
"github.com/sourcegraph/src-cli/internal/batches"
8+
"github.com/sourcegraph/src-cli/internal/batches/service"
99
"github.com/sourcegraph/src-cli/internal/output"
1010
)
1111

@@ -41,7 +41,7 @@ Examples:
4141
}
4242
defer specFile.Close()
4343

44-
svc := batches.NewService(&batches.ServiceOpts{})
44+
svc := service.New(&service.Opts{})
4545

4646
out := output.NewOutput(flagSet.Output(), output.OutputOpts{Verbose: *verbose})
4747
if _, _, err := batchParseSpec(out, svc, specFile); err != nil {

‎internal/batches/batch_spec.go‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package batches
22

33
import (
4+
"context"
45
"fmt"
56
"strings"
67

@@ -72,6 +73,14 @@ type WorkspaceConfiguration struct {
7273
glob glob.Glob
7374
}
7475

76+
func (wc *WorkspaceConfiguration) SetGlob(g glob.Glob) {
77+
wc.glob = g
78+
}
79+
80+
func (wc *WorkspaceConfiguration) Matches(repoName string) bool {
81+
return wc.glob.Match(repoName)
82+
}
83+
7584
type OnQueryOrRepository struct {
7685
RepositoriesMatchingQuery string `json:"repositoriesMatchingQuery,omitempty" yaml:"repositoriesMatchingQuery"`
7786
Repository string `json:"repository,omitempty" yaml:"repository"`
@@ -88,6 +97,27 @@ type Step struct {
8897
image docker.Image
8998
}
9099

100+
func (s *Step) SetImage(image docker.Image) {
101+
s.image = image
102+
}
103+
104+
// TODO(mrnugget): All of these wrappers are not good
105+
func (s Step) ImageDigest(ctx context.Context) (string, error) {
106+
return s.image.Digest(ctx)
107+
}
108+
109+
func (s Step) DockerImage() docker.Image {
110+
return s.image
111+
}
112+
113+
func (s Step) EnsureImage(ctx context.Context) error {
114+
return s.image.Ensure(ctx)
115+
}
116+
117+
func (s Step) ImageUIDGID(ctx context.Context) (docker.UIDGID, error) {
118+
return s.image.UIDGID(ctx)
119+
}
120+
91121
type Outputs map[string]Output
92122

93123
type Output struct {
@@ -105,7 +135,7 @@ type Group struct {
105135
Repository string `json:"repository,omitempty" yaml:"repository"`
106136
}
107137

108-
func ParseBatchSpec(data []byte, features featureFlags) (*BatchSpec, error) {
138+
func ParseBatchSpec(data []byte, features FeatureFlags) (*BatchSpec, error) {
109139
var spec BatchSpec
110140
if err := yaml.UnmarshalValidate(schema.BatchSpecJSON, data, &spec); err != nil {
111141
if multiErr, ok := err.(*multierror.Error); ok {
@@ -128,7 +158,7 @@ func ParseBatchSpec(data []byte, features featureFlags) (*BatchSpec, error) {
128158

129159
var errs *multierror.Error
130160

131-
if !features.allowArrayEnvironments {
161+
if !features.AllowArrayEnvironments {
132162
for i, step := range spec.Steps {
133163
if !step.Env.IsStatic() {
134164
errs = multierror.Append(errs, errors.Errorf("step %d includes one or more dynamic environment variables, which are unsupported in this Sourcegraph version", i+1))
@@ -140,11 +170,11 @@ func ParseBatchSpec(data []byte, features featureFlags) (*BatchSpec, error) {
140170
errs = multierror.Append(errs, errors.New("batch spec includes steps but no changesetTemplate"))
141171
}
142172

143-
if spec.TransformChanges != nil && !features.allowtransformChanges {
173+
if spec.TransformChanges != nil && !features.AllowTransformChanges {
144174
errs = multierror.Append(errs, errors.New("batch spec includes transformChanges, which is not supported in this Sourcegraph version"))
145175
}
146176

147-
if len(spec.Workspaces) != 0 && !features.allowtransformChanges {
177+
if len(spec.Workspaces) != 0 && !features.AllowTransformChanges {
148178
errs = multierror.Append(errs, errors.New("batch spec includes workspaces, which is not supported in this Sourcegraph version"))
149179
}
150180

‎internal/batches/batch_spec_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ changesetTemplate:
2121
published: false
2222
`
2323

24-
_, err := ParseBatchSpec([]byte(spec), featureFlags{})
24+
_, err := ParseBatchSpec([]byte(spec), FeatureFlags{})
2525
if err != nil {
2626
t.Fatalf("parsing valid spec returned error: %s", err)
2727
}
@@ -38,7 +38,7 @@ steps:
3838
container: alpine:3
3939
`
4040

41-
_, err := ParseBatchSpec([]byte(spec), featureFlags{})
41+
_, err := ParseBatchSpec([]byte(spec), FeatureFlags{})
4242
if err == nil {
4343
t.Fatal("no error returned")
4444
}
@@ -71,7 +71,7 @@ changesetTemplate:
7171
published: false
7272
`
7373

74-
_, err := ParseBatchSpec([]byte(spec), featureFlags{})
74+
_, err := ParseBatchSpec([]byte(spec), FeatureFlags{})
7575
if err == nil {
7676
t.Fatal("no error returned")
7777
}

0 commit comments

Comments
 (0)