From 175628eb174eb3183d047bd166cbb6a3b1836b82 Mon Sep 17 00:00:00 2001 From: Marc <7050295+marcleblanc2@users.noreply.github.com> Date: Tue, 10 Dec 2024 05:14:41 -0700 Subject: [PATCH] fix: batches now respects skip-errors for version check (#1135) * fix: batches now respects skip-errors for version check Co-authored-by: Michael Bahr --- 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 | 11 +++++++++-- internal/batches/service/service.go | 4 ++-- 7 files changed, 74 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..228775f66a 100644 --- a/internal/batches/features.go +++ b/internal/batches/features.go @@ -1,6 +1,9 @@ package batches import ( + "fmt" + "log" + "github.com/sourcegraph/sourcegraph/lib/api" "github.com/sourcegraph/sourcegraph/lib/errors" ) @@ -12,7 +15,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 +35,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) }