Remove diag.SafeDiagnostics; replace with logdiag (#3197)

logdiag is thread-safe as well and easier to use

Author: denik

bundle/apps/upload_config.go

@@ -12,6 +12,7 @@ import (
 	"github.com/databricks/cli/bundle/deploy"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/filer"
+	"github.com/databricks/cli/libs/logdiag"
 	"golang.org/x/sync/errgroup"
 
 	"gopkg.in/yaml.v3"
@@ -22,7 +23,6 @@ type uploadConfig struct {
 }
 
 func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
-	var diags diag.SafeDiagnostics
 	errGroup, ctx := errgroup.WithContext(ctx)
 
 	for key, app := range b.Config.Resources.Apps {
@@ -45,7 +45,7 @@ func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos
 			errGroup.Go(func() error {
 				err := f.Write(ctx, path.Join(appPath, "app.yml"), buf, filer.OverwriteIfExists)
 				if err != nil {
-					diags.Append(diag.Diagnostic{
+					logdiag.LogDiag(ctx, diag.Diagnostic{
 						Severity:  diag.Error,
 						Summary:   "Failed to save config",
 						Detail:    fmt.Sprintf("Failed to write %s file: %s", path.Join(app.SourceCodePath, "app.yml"), err),
@@ -58,10 +58,10 @@ func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos
 	}
 
 	if err := errGroup.Wait(); err != nil {
-		diags.AppendError(err)
+		logdiag.LogError(ctx, err)
 	}
 
-	return diags.Diags
+	return nil
 }
 
 // Name implements bundle.Mutator.

bundle/config/validate/validate_sync_patterns.go

@@ -9,6 +9,7 @@ import (
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/fileset"
+	"github.com/databricks/cli/libs/logdiag"
 	"golang.org/x/sync/errgroup"
 )
 
@@ -24,26 +25,18 @@ func (v *validateSyncPatterns) Name() string {
 
 func (v *validateSyncPatterns) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	s := b.Config.Sync
-	if len(s.Exclude) == 0 && len(s.Include) == 0 {
-		return nil
-	}
-
-	diags, err := checkPatterns(s.Exclude, "sync.exclude", b)
-	if err != nil {
-		return diag.FromErr(err)
-	}
 
-	includeDiags, err := checkPatterns(s.Include, "sync.include", b)
-	if err != nil {
-		return diag.FromErr(err)
+	checkPatterns(ctx, s.Exclude, "sync.exclude", b)
+	if logdiag.HasError(ctx) {
+		return nil
 	}
 
-	return diags.Extend(includeDiags)
+	checkPatterns(ctx, s.Include, "sync.include", b)
+	return nil
 }
 
-func checkPatterns(patterns []string, path string, b *bundle.Bundle) (diag.Diagnostics, error) {
+func checkPatterns(ctx context.Context, patterns []string, path string, b *bundle.Bundle) {
 	var errs errgroup.Group
-	var diags diag.SafeDiagnostics
 
 	for index, pattern := range patterns {
 		// If the pattern is negated, strip the negation prefix
@@ -65,7 +58,7 @@ func checkPatterns(patterns []string, path string, b *bundle.Bundle) (diag.Diagn
 
 			if len(all) == 0 {
 				path := fmt.Sprintf("%s[%d]", path, index)
-				diags.Append(diag.Diagnostic{
+				logdiag.LogDiag(ctx, diag.Diagnostic{
 					Severity:  diag.Warning,
 					Summary:   fmt.Sprintf("Pattern %s does not match any files", pattern),
 					Locations: b.Config.GetLocations(path),
@@ -76,5 +69,8 @@ func checkPatterns(patterns []string, path string, b *bundle.Bundle) (diag.Diagn
 		})
 	}
 
-	return diags.Diags, errs.Wait()
+	err := errs.Wait()
+	if err != nil {
+		logdiag.LogError(ctx, err)
+	}
 }

bundle/terranova/apply.go

@@ -15,6 +15,7 @@ import (
 	"github.com/databricks/cli/libs/dagrun"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/log"
+	"github.com/databricks/cli/libs/logdiag"
 	"github.com/databricks/cli/libs/structdiff"
 	"github.com/databricks/databricks-sdk-go"
 )
@@ -46,14 +47,13 @@ func (m *terranovaApplyMutator) Apply(ctx context.Context, b *bundle.Bundle) dia
 	}
 
 	client := b.WorkspaceClient()
-	var diags diag.SafeDiagnostics
 
 	err := g.Run(defaultParallelism, func(node deployplan.Action) {
 		// TODO: if a given node fails, all downstream nodes should not be run. We should report those nodes.
 		// TODO: ensure that config for this node is fully resolved at this point.
 
 		if node.ActionType == deployplan.ActionTypeUnset {
-			diags.AppendErrorf("internal error, no action set %#v", node)
+			logdiag.LogError(ctx, fmt.Errorf("internal error, no action set %#v", node))
 			return
 		}
 
@@ -63,7 +63,7 @@ func (m *terranovaApplyMutator) Apply(ctx context.Context, b *bundle.Bundle) dia
 			var ok bool
 			config, ok = b.GetResourceConfig(node.Group, node.Name)
 			if !ok {
-				diags.AppendErrorf("internal error: cannot get config for group=%v name=%v", node.Group, node.Name)
+				logdiag.LogError(ctx, fmt.Errorf("internal error: cannot get config for group=%v name=%v", node.Group, node.Name))
 				return
 			}
 		}
@@ -78,20 +78,20 @@ func (m *terranovaApplyMutator) Apply(ctx context.Context, b *bundle.Bundle) dia
 		// TODO: pass node.ActionType and respect it. Do not change Update to Recreate or Delete for example.
 		err := d.Deploy(ctx, config, node.ActionType)
 		if err != nil {
-			diags.AppendError(err)
+			logdiag.LogError(ctx, err)
 			return
 		}
 	})
 	if err != nil {
-		diags.AppendError(err)
+		logdiag.LogError(ctx, err)
 	}
 
 	err = b.ResourceDatabase.Finalize()
 	if err != nil {
-		diags.AppendError(err)
+		logdiag.LogError(ctx, err)
 	}
 
-	return diags.Diags
+	return nil
 }
 
 type Deployer struct {

libs/diag/diagnostic.go

@@ -3,7 +3,6 @@ package diag
 import (
 	"errors"
 	"fmt"
-	"sync"
 
 	"github.com/databricks/cli/libs/dyn"
 )
@@ -147,30 +146,3 @@ func (ds Diagnostics) Filter(severity Severity) Diagnostics {
 	}
 	return out
 }
-
-type SafeDiagnostics struct {
-	Mutex sync.Mutex
-	Diags Diagnostics
-}
-
-func (t *SafeDiagnostics) Append(d Diagnostic) {
-	t.Mutex.Lock()
-	defer t.Mutex.Unlock()
-
-	t.Diags = t.Diags.Append(d)
-}
-
-func (t *SafeDiagnostics) Extend(other Diagnostics) {
-	t.Mutex.Lock()
-	defer t.Mutex.Unlock()
-
-	t.Diags = t.Diags.Extend(other)
-}
-
-func (t *SafeDiagnostics) AppendErrorf(msg string, args ...any) {
-	t.Extend(FromErr(fmt.Errorf(msg, args...)))
-}
-
-func (t *SafeDiagnostics) AppendError(err error) {
-	t.Extend(FromErr(err))
-}