fix: batches now respects skip-errors for version check (#1135)

* fix: batches now respects skip-errors for version check
Co-authored-by: Michael Bahr <michael.bahr@sourcegraph.com>

Author: marcleblanc2

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
 	}

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 {

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})

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})

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)

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
 	}

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)
 
 }