fix: Quality of life improvements (#172)

Extracted from #170

.github/workflows/lint_golang.yml

@@ -31,5 +31,5 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.54.2
+          version: v1.55.2
           args: --verbose

.golangci.yml

@@ -51,6 +51,10 @@ linters-settings:
         disabled: true
       - name: nested-structs
         disabled: true
+      - name: import-alias-naming
+        disabled: true
+      - name: unchecked-type-assertion
+        disabled: true
 
   gofmt:
     rewrite-rules:

go.mod

@@ -14,6 +14,7 @@ require (
 	github.com/opencontainers/image-spec v1.0.2
 	github.com/rs/zerolog v1.29.1
 	github.com/schollz/progressbar/v3 v3.13.1
+	github.com/stretchr/testify v1.8.4
 	github.com/thoas/go-funk v0.9.3
 	golang.org/x/exp v0.0.0-20231006140011-7918f672742d
 	google.golang.org/grpc v1.58.3
@@ -36,6 +37,7 @@ require (
 	github.com/bytedance/sonic v1.10.2 // indirect
 	github.com/chenzhuoyu/base64x v0.0.0-20230717121745-296ad89f973d // indirect
 	github.com/chenzhuoyu/iasm v0.9.0 // indirect
+	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/deepmap/oapi-codegen v1.15.0 // indirect
 	github.com/distribution/reference v0.5.0 // indirect
 	github.com/docker/distribution v2.8.3+incompatible // indirect
@@ -84,6 +86,7 @@ require (
 	github.com/pelletier/go-toml/v2 v2.0.8 // indirect
 	github.com/pierrec/lz4/v4 v4.1.18 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/rivo/uniseg v0.2.0 // indirect
 	github.com/russross/blackfriday/v2 v2.1.0 // indirect
 	github.com/schollz/closestmatch v2.1.0+incompatible // indirect

managedplugin/download.go

@@ -85,20 +85,21 @@ func getURLLocation(ctx context.Context, org string, name string, version string
 			}
 			resp.Body.Close()
 			// Check server response
-			switch {
-			case resp.StatusCode == http.StatusNotFound:
+			switch resp.StatusCode {
+			case http.StatusOK:
+				return nil
+			case http.StatusNotFound:
 				return err404
-			case resp.StatusCode == http.StatusUnauthorized:
+			case http.StatusUnauthorized:
 				fmt.Printf("Failed downloading %s with status code %d. Retrying\n", downloadURL, resp.StatusCode)
 				return err401
-			case resp.StatusCode == http.StatusTooManyRequests:
+			case http.StatusTooManyRequests:
 				fmt.Printf("Failed downloading %s with status code %d. Retrying\n", downloadURL, resp.StatusCode)
 				return err429
-			case resp.StatusCode >= http.StatusBadRequest: // anything that's not 200 or 30*
+			default:
 				fmt.Printf("Failed downloading %s with status code %d\n", downloadURL, resp.StatusCode)
 				return fmt.Errorf("statusCode %d", resp.StatusCode)
 			}
-			return nil
 		}, retry.RetryIf(func(err error) bool {
 			return err == err401 || err == err429
 		}),

managedplugin/plugin.go

@@ -96,13 +96,13 @@ type Client struct {
 
 // typ will be deprecated soon but now required for a transition period
 func NewClients(ctx context.Context, typ PluginType, specs []Config, opts ...Option) (Clients, error) {
-	clients := make(Clients, len(specs))
-	for i, spec := range specs {
+	clients := make(Clients, 0, len(specs))
+	for _, spec := range specs {
 		client, err := NewClient(ctx, typ, spec, opts...)
 		if err != nil {
-			return nil, err
+			return clients, err // previous entries in clients determine which plugins were successfully created
 		}
-		clients[i] = client
+		clients = append(clients, client)
 	}
 	return clients, nil
 }

managedplugin/registry_test.go

@@ -17,11 +17,11 @@ func TestRegistryJsonMarshalUnmarshal(t *testing.T) {
 		t.Fatal("failed to unmarshal registry:", err)
 	}
 	if registry != RegistryGrpc {
-		t.Fatal("expected registry to be github, but got:", registry)
+		t.Fatal("expected registry to be grpc, but got:", registry)
 	}
 }
 
-func TestRegistryYamlMarshalUnmarsahl(t *testing.T) {
+func TestRegistryYamlMarshalUnmarshal(t *testing.T) {
 	b, err := yaml.Marshal(RegistryGrpc)
 	if err != nil {
 		t.Fatal("failed to marshal registry:", err)
@@ -31,6 +31,6 @@ func TestRegistryYamlMarshalUnmarsahl(t *testing.T) {
 		t.Fatal("failed to unmarshal registry:", err)
 	}
 	if registry != RegistryGrpc {
-		t.Fatal("expected registry to be github, but got:", registry)
+		t.Fatal("expected registry to be grpc, but got:", registry)
 	}
 }

specs/destination.go

@@ -23,9 +23,6 @@ type Destination struct {
 }
 
 func (d *Destination) SetDefaults(defaultBatchSize, defaultBatchSizeBytes int) {
-	if d.Registry.String() == "" {
-		d.Registry = RegistryGithub
-	}
 	if d.BatchSize == 0 {
 		d.BatchSize = defaultBatchSize
 	}
@@ -59,7 +56,7 @@ func (d *Destination) Validate() error {
 		return fmt.Errorf(msg)
 	}
 
-	if d.Registry == RegistryGithub {
+	if d.Registry.NeedVersion() {
 		if d.Version == "" {
 			return fmt.Errorf("version is required")
 		}

specs/destination_test.go

@@ -3,7 +3,7 @@ package specs
 import (
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/require"
 )
 
 type testDestinationSpec struct {
@@ -77,9 +77,7 @@ func TestDestinationUnmarshalSpec(t *testing.T) {
 			}
 
 			source := spec.Spec.(*Source)
-			if cmp.Diff(source, tc.source) != "" {
-				t.Fatalf("expected:%v got:%v", tc.source, source)
-			}
+			require.Equal(t, tc.source, source)
 		})
 	}
 }
@@ -191,9 +189,7 @@ func TestDestinationUnmarshalSpecValidate(t *testing.T) {
 				return
 			}
 
-			if cmp.Diff(destination, tc.destination) != "" {
-				t.Fatalf("expected:\n%v\ngot:\n%v\n", tc.destination, destination)
-			}
+			require.Equal(t, tc.destination, destination)
 		})
 	}
 }

specs/registry.go

@@ -37,6 +37,10 @@ func (r *Registry) UnmarshalJSON(data []byte) (err error) {
 	return nil
 }
 
+func (r Registry) NeedVersion() bool {
+	return r == RegistryGithub || r == RegistryCloudQuery
+}
+
 func RegistryFromString(s string) (Registry, error) {
 	switch s {
 	case "github":

specs/registry_test.go

@@ -17,11 +17,11 @@ func TestRegistryJsonMarshalUnmarshal(t *testing.T) {
 		t.Fatal("failed to unmarshal registry:", err)
 	}
 	if registry != RegistryGrpc {
-		t.Fatal("expected registry to be github, but got:", registry)
+		t.Fatal("expected registry to be grpc, but got:", registry)
 	}
 }
 
-func TestRegistryYamlMarshalUnmarsahl(t *testing.T) {
+func TestRegistryYamlMarshalUnmarshal(t *testing.T) {
 	b, err := yaml.Marshal(RegistryGrpc)
 	if err != nil {
 		t.Fatal("failed to marshal registry:", err)
@@ -31,6 +31,6 @@ func TestRegistryYamlMarshalUnmarsahl(t *testing.T) {
 		t.Fatal("failed to unmarshal registry:", err)
 	}
 	if registry != RegistryGrpc {
-		t.Fatal("expected registry to be github, but got:", registry)
+		t.Fatal("expected registry to be grpc, but got:", registry)
 	}
 }

specs/source.go

@@ -25,7 +25,7 @@ type Source struct {
 	// For the local registry the path will be the path to the binary: ./path/to/binary
 	// For the gRPC registry the path will be the address of the gRPC server: host:port
 	Path string `json:"path,omitempty"`
-	// Registry can be github,local,grpc.
+	// Registry can be github,local,grpc,cloudquery
 	Registry            Registry `json:"registry,omitempty"`
 	Concurrency         uint64   `json:"concurrency,omitempty"`
 	TableConcurrency    uint64   `json:"table_concurrency,omitempty"`    // deprecated: use Concurrency instead
@@ -55,9 +55,6 @@ type Source struct {
 }
 
 func (s *Source) SetDefaults() {
-	if s.Registry.String() == "" {
-		s.Registry = RegistryGithub
-	}
 	if s.Backend.String() == "" {
 		s.Backend = BackendNone
 	}
@@ -117,7 +114,7 @@ func (s *Source) Validate() error {
 		return fmt.Errorf("tables configuration is required. Hint: set the tables you want to sync by adding `tables: [...]` or use `cloudquery tables` to list available tables")
 	}
 
-	if s.Registry == RegistryGithub {
+	if s.Registry.NeedVersion() {
 		if s.Version == "" {
 			return fmt.Errorf("version is required")
 		}

specs/source_test.go

@@ -3,7 +3,7 @@ package specs
 import (
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/require"
 )
 
 var sourceUnmarshalSpecTestCases = []struct {
@@ -58,9 +58,7 @@ func TestSourceUnmarshalSpec(t *testing.T) {
 			}
 
 			source := spec.Spec.(*Source)
-			if cmp.Diff(source, tc.source) != "" {
-				t.Fatalf("expected:%v got:%v", tc.source, source)
-			}
+			require.Equal(t, tc.source, source)
 		})
 	}
 }
@@ -199,9 +197,7 @@ func TestSourceUnmarshalSpecValidate(t *testing.T) {
 				return
 			}
 
-			if cmp.Diff(source, tc.source) != "" {
-				t.Fatalf("expected:%v got:%v", tc.source, source)
-			}
+			require.Equal(t, tc.source, source)
 		})
 	}
 }