From 93a3106f51c539f2bcda1a6b7f7fb0bf8620001c Mon Sep 17 00:00:00 2001 From: Michael Bahr Date: Tue, 19 Nov 2024 10:49:48 +0100 Subject: [PATCH] fix: batches now respects skip-errors for version check --- cmd/src/batch_common.go | 23 ++++++++++++++--------- cmd/src/batch_new.go | 18 ++++++++++++++---- cmd/src/batch_remote.go | 11 ++++++++--- cmd/src/batch_repositories.go | 16 +++++++++++++--- cmd/src/batch_validate.go | 18 ++++++++++++++---- internal/batches/features.go | 10 ++++++++-- internal/batches/service/service.go | 4 ++-- 7 files changed, 73 insertions(+), 27 deletions(-) diff --git a/cmd/src/batch_common.go b/cmd/src/batch_common.go index 57b248cf06..8171156675 100644 --- a/cmd/src/batch_common.go +++ b/cmd/src/batch_common.go @@ -5,6 +5,7 @@ import ( "flag" "fmt" "io" + cliLog "log" "os" "os/exec" "os/signal" @@ -48,6 +49,7 @@ type batchExecutionFlags struct { api *api.Flags clearCache bool namespace string + skipErrors bool } func newBatchExecutionFlags(flagSet *flag.FlagSet) *batchExecutionFlags { @@ -63,6 +65,10 @@ func newBatchExecutionFlags(flagSet *flag.FlagSet) *batchExecutionFlags { &bef.clearCache, "clear-cache", false, "If true, clears the execution cache and executes all steps anew.", ) + flagSet.BoolVar( + &bef.skipErrors, "skip-errors", false, + "If true, errors encountered won't stop the program, but only log them.", + ) flagSet.BoolVar( &bef.allowIgnored, "force-override-ignore", false, "Do not ignore repositories that have a .batchignore file.", @@ -146,11 +152,6 @@ func newBatchExecuteFlags(flagSet *flag.FlagSet, cacheDir, tempDir string) *batc "If true, deletes downloaded repository archives after executing batch spec steps. Note that only the archives related to the actual repositories matched by the batch spec will be cleaned up, and clean up will not occur if src exits unexpectedly.", ) - flagSet.BoolVar( - &caf.skipErrors, "skip-errors", false, - "If true, errors encountered while executing steps in a repository won't stop the execution of the batch spec but only cause that repository to be skipped.", - ) - flagSet.StringVar( &caf.workspace, "workspace", "auto", `Workspace mode to use ("auto", "bind", or "volume")`, @@ -290,7 +291,7 @@ func executeBatchSpec(ctx context.Context, opts executeBatchSpecOpts) (err error Client: opts.client, }) - lr, ffs, err := svc.DetermineLicenseAndFeatureFlags(ctx) + lr, ffs, err := svc.DetermineLicenseAndFeatureFlags(ctx, opts.flags.skipErrors) if err != nil { return err } @@ -302,8 +303,12 @@ func executeBatchSpec(ctx context.Context, opts executeBatchSpecOpts) (err error imageCache := docker.NewImageCache() - if err := validateSourcegraphVersionConstraint(ctx, ffs); err != nil { - return err + if err := validateSourcegraphVersionConstraint(ffs); err != nil { + if !opts.flags.skipErrors { + return err + } else { + cliLog.Printf("WARNING: %s", err) + } } if err := checkExecutable("git", "version"); err != nil { @@ -655,7 +660,7 @@ func getBatchParallelism(ctx context.Context, flag int) (int, error) { return docker.NCPU(ctx) } -func validateSourcegraphVersionConstraint(ctx context.Context, ffs *batches.FeatureFlags) error { +func validateSourcegraphVersionConstraint(ffs *batches.FeatureFlags) error { if ffs.Sourcegraph40 { return nil } diff --git a/cmd/src/batch_new.go b/cmd/src/batch_new.go index 349b590eff..2dc4b7a5e5 100644 --- a/cmd/src/batch_new.go +++ b/cmd/src/batch_new.go @@ -4,6 +4,7 @@ import ( "context" "flag" "fmt" + cliLog "log" "github.com/sourcegraph/src-cli/internal/api" "github.com/sourcegraph/src-cli/internal/batches/service" @@ -30,7 +31,12 @@ Examples: apiFlags := api.NewFlags(flagSet) var ( - fileFlag = flagSet.String("f", "batch.yaml", "The name of the batch spec file to create.") + fileFlag = flagSet.String("f", "batch.yaml", "The name of the batch spec file to create.") + skipErrors bool + ) + flagSet.BoolVar( + &skipErrors, "skip-errors", false, + "If true, errors encountered won't stop the program, but only log them.", ) handler := func(args []string) error { @@ -48,13 +54,17 @@ Examples: Client: cfg.apiClient(apiFlags, flagSet.Output()), }) - _, ffs, err := svc.DetermineLicenseAndFeatureFlags(ctx) + _, ffs, err := svc.DetermineLicenseAndFeatureFlags(ctx, skipErrors) if err != nil { return err } - if err := validateSourcegraphVersionConstraint(ctx, ffs); err != nil { - return err + if err := validateSourcegraphVersionConstraint(ffs); err != nil { + if !skipErrors { + return err + } else { + cliLog.Printf("WARNING: %s", err) + } } if err := svc.GenerateExampleSpec(ctx, *fileFlag); err != nil { diff --git a/cmd/src/batch_remote.go b/cmd/src/batch_remote.go index 7817421ab3..7dd7628c03 100644 --- a/cmd/src/batch_remote.go +++ b/cmd/src/batch_remote.go @@ -4,6 +4,7 @@ import ( "context" "flag" "fmt" + cliLog "log" "strings" "time" @@ -52,13 +53,17 @@ Examples: Client: cfg.apiClient(flags.api, flagSet.Output()), }) - _, ffs, err := svc.DetermineLicenseAndFeatureFlags(ctx) + _, ffs, err := svc.DetermineLicenseAndFeatureFlags(ctx, flags.skipErrors) if err != nil { return err } - if err := validateSourcegraphVersionConstraint(ctx, ffs); err != nil { - return err + if err := validateSourcegraphVersionConstraint(ffs); err != nil { + if !flags.skipErrors { + return err + } else { + cliLog.Printf("WARNING: %s", err) + } } out := output.NewOutput(flagSet.Output(), output.OutputOpts{Verbose: *verbose}) diff --git a/cmd/src/batch_repositories.go b/cmd/src/batch_repositories.go index 092ae637d2..b02a4f5e58 100644 --- a/cmd/src/batch_repositories.go +++ b/cmd/src/batch_repositories.go @@ -4,6 +4,7 @@ import ( "context" "flag" "fmt" + cliLog "log" "github.com/sourcegraph/sourcegraph/lib/errors" "github.com/sourcegraph/sourcegraph/lib/output" @@ -42,6 +43,7 @@ Examples: var ( allowUnsupported bool allowIgnored bool + skipErrors bool ) flagSet.BoolVar( &allowUnsupported, "allow-unsupported", false, @@ -51,6 +53,10 @@ Examples: &allowIgnored, "force-override-ignore", false, "Do not ignore repositories that have a .batchignore file.", ) + flagSet.BoolVar( + &skipErrors, "skip-errors", false, + "If true, errors encountered won't stop the program, but only log them.", + ) handler := func(args []string) error { if err := flagSet.Parse(args); err != nil { @@ -69,13 +75,17 @@ Examples: Client: client, }) - _, ffs, err := svc.DetermineLicenseAndFeatureFlags(ctx) + _, ffs, err := svc.DetermineLicenseAndFeatureFlags(ctx, skipErrors) if err != nil { return err } - if err := validateSourcegraphVersionConstraint(ctx, ffs); err != nil { - return err + if err := validateSourcegraphVersionConstraint(ffs); err != nil { + if !skipErrors { + return err + } else { + cliLog.Printf("WARNING: %s", err) + } } out := output.NewOutput(flagSet.Output(), output.OutputOpts{Verbose: *verbose}) diff --git a/cmd/src/batch_validate.go b/cmd/src/batch_validate.go index e2c77c29bc..5f09caeebc 100644 --- a/cmd/src/batch_validate.go +++ b/cmd/src/batch_validate.go @@ -4,6 +4,7 @@ import ( "context" "flag" "fmt" + cliLog "log" "github.com/sourcegraph/sourcegraph/lib/output" @@ -36,6 +37,7 @@ Examples: var ( allowUnsupported bool allowIgnored bool + skipErrors bool ) flagSet.BoolVar( &allowUnsupported, "allow-unsupported", false, @@ -45,6 +47,10 @@ Examples: &allowIgnored, "force-override-ignore", false, "Do not ignore repositories that have a .batchignore file.", ) + flagSet.BoolVar( + &skipErrors, "skip-errors", false, + "If true, errors encountered won't stop the program, but only log them.", + ) handler := func(args []string) error { ctx := context.Background() @@ -63,14 +69,18 @@ Examples: Client: cfg.apiClient(apiFlags, flagSet.Output()), }) - _, ffs, err := svc.DetermineLicenseAndFeatureFlags(ctx) + _, ffs, err := svc.DetermineLicenseAndFeatureFlags(ctx, skipErrors) if err != nil { return err } - if err := validateSourcegraphVersionConstraint(ctx, ffs); err != nil { - ui.ExecutionError(err) - return err + if err := validateSourcegraphVersionConstraint(ffs); err != nil { + if !skipErrors { + ui.ExecutionError(err) + return err + } else { + cliLog.Printf("WARNING: %s", err) + } } file, err := getBatchSpecFile(flagSet, fileFlag) diff --git a/internal/batches/features.go b/internal/batches/features.go index ee319709b7..4098fd9234 100644 --- a/internal/batches/features.go +++ b/internal/batches/features.go @@ -1,8 +1,10 @@ package batches import ( + "fmt" "github.com/sourcegraph/sourcegraph/lib/api" "github.com/sourcegraph/sourcegraph/lib/errors" + "log" ) // FeatureFlags represent features that are only available on certain @@ -12,7 +14,7 @@ type FeatureFlags struct { BinaryDiffs bool } -func (ff *FeatureFlags) SetFromVersion(version string) error { +func (ff *FeatureFlags) SetFromVersion(version string, skipErrors bool) error { for _, feature := range []struct { flag *bool constraint string @@ -32,7 +34,11 @@ func (ff *FeatureFlags) SetFromVersion(version string) error { } { value, err := api.CheckSourcegraphVersion(version, feature.constraint, feature.minDate) if err != nil { - return errors.Wrap(err, "failed to check version returned by Sourcegraph") + if skipErrors { + log.Printf("failed to check version returned by Sourcegraph: %s. Assuming no feature flags.", version) + } else { + return errors.Wrap(err, fmt.Sprintf("failed to check version returned by Sourcegraph: %s", version)) + } } *feature.flag = value } diff --git a/internal/batches/service/service.go b/internal/batches/service/service.go index 8fb1116d1f..9c8339e3c3 100644 --- a/internal/batches/service/service.go +++ b/internal/batches/service/service.go @@ -76,7 +76,7 @@ func (svc *Service) getSourcegraphVersionAndMaxChangesetsCount(ctx context.Conte // DetermineLicenseAndFeatureFlags returns the enabled features and license restrictions // configured for the Sourcegraph instance. -func (svc *Service) DetermineLicenseAndFeatureFlags(ctx context.Context) (*batches.LicenseRestrictions, *batches.FeatureFlags, error) { +func (svc *Service) DetermineLicenseAndFeatureFlags(ctx context.Context, skipErrors bool) (*batches.LicenseRestrictions, *batches.FeatureFlags, error) { version, mc, err := svc.getSourcegraphVersionAndMaxChangesetsCount(ctx) if err != nil { return nil, nil, errors.Wrap(err, "failed to query Sourcegraph version and license info for instance") @@ -87,7 +87,7 @@ func (svc *Service) DetermineLicenseAndFeatureFlags(ctx context.Context) (*batch } ffs := &batches.FeatureFlags{} - return lr, ffs, ffs.SetFromVersion(version) + return lr, ffs, ffs.SetFromVersion(version, skipErrors) }