Skip to content

Commit ed19466

Browse files
pieternpietern
authored
Return diag.Diagnostics from mutators (#1305)
## Changes This diagnostics type allows us to capture multiple warnings as well as errors in the return value. This is a preparation for returning additional warnings from mutators in case we detect non-fatal problems. * All return statements that previously returned an error now return `diag.FromErr` * All return statements that previously returned `fmt.Errorf` now return `diag.Errorf` * All `err != nil` checks now use `diags.HasError()` or `diags.Error()` ## Tests * Existing tests pass. * I confirmed no call site under `./bundle` or `./cmd/bundle` uses `errors.Is` on the return value from mutators. This is relevant because we cannot wrap errors with `%w` when calling `diag.Errorf` (like `fmt.Errorf`; context in golang/go#47641).
1 parent 9cf3dbe commit ed19466

File tree

141 files changed

+841
-698
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

141 files changed

+841
-698
lines changed

‎bundle/artifacts/all.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"slices"
88

99
"github.com/databricks/cli/bundle"
10+
"github.com/databricks/cli/libs/diag"
1011
"golang.org/x/exp/maps"
1112
)
1213

@@ -21,7 +22,7 @@ func (m *all) Name() string {
2122
return fmt.Sprintf("artifacts.%sAll", m.name)
2223
}
2324

24-
func (m *all) Apply(ctx context.Context, b *bundle.Bundle) error {
25+
func (m *all) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
2526
var out []bundle.Mutator
2627

2728
// Iterate with stable ordering.
@@ -31,7 +32,7 @@ func (m *all) Apply(ctx context.Context, b *bundle.Bundle) error {
3132
for _, name := range keys {
3233
m, err := m.fn(name)
3334
if err != nil {
34-
return err
35+
return diag.FromErr(err)
3536
}
3637
if m != nil {
3738
out = append(out, m)

‎bundle/artifacts/artifacts.go‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/databricks/cli/bundle/config"
1515
"github.com/databricks/cli/bundle/libraries"
1616
"github.com/databricks/cli/libs/cmdio"
17+
"github.com/databricks/cli/libs/diag"
1718
"github.com/databricks/cli/libs/filer"
1819
"github.com/databricks/cli/libs/log"
1920
)
@@ -57,17 +58,17 @@ func (m *basicBuild) Name() string {
5758
return fmt.Sprintf("artifacts.Build(%s)", m.name)
5859
}
5960

60-
func (m *basicBuild) Apply(ctx context.Context, b *bundle.Bundle) error {
61+
func (m *basicBuild) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
6162
artifact, ok := b.Config.Artifacts[m.name]
6263
if !ok {
63-
return fmt.Errorf("artifact doesn't exist: %s", m.name)
64+
return diag.Errorf("artifact doesn't exist: %s", m.name)
6465
}
6566

6667
cmdio.LogString(ctx, fmt.Sprintf("Building %s...", m.name))
6768

6869
out, err := artifact.Build(ctx)
6970
if err != nil {
70-
return fmt.Errorf("build for %s failed, error: %w, output: %s", m.name, err, out)
71+
return diag.Errorf("build for %s failed, error: %v, output: %s", m.name, err, out)
7172
}
7273
log.Infof(ctx, "Build succeeded")
7374

@@ -87,29 +88,29 @@ func (m *basicUpload) Name() string {
8788
return fmt.Sprintf("artifacts.Upload(%s)", m.name)
8889
}
8990

90-
func (m *basicUpload) Apply(ctx context.Context, b *bundle.Bundle) error {
91+
func (m *basicUpload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
9192
artifact, ok := b.Config.Artifacts[m.name]
9293
if !ok {
93-
return fmt.Errorf("artifact doesn't exist: %s", m.name)
94+
return diag.Errorf("artifact doesn't exist: %s", m.name)
9495
}
9596

9697
if len(artifact.Files) == 0 {
97-
return fmt.Errorf("artifact source is not configured: %s", m.name)
98+
return diag.Errorf("artifact source is not configured: %s", m.name)
9899
}
99100

100101
uploadPath, err := getUploadBasePath(b)
101102
if err != nil {
102-
return err
103+
return diag.FromErr(err)
103104
}
104105

105106
client, err := filer.NewWorkspaceFilesClient(b.WorkspaceClient(), uploadPath)
106107
if err != nil {
107-
return err
108+
return diag.FromErr(err)
108109
}
109110

110111
err = uploadArtifact(ctx, b, artifact, uploadPath, client)
111112
if err != nil {
112-
return fmt.Errorf("upload for %s failed, error: %w", m.name, err)
113+
return diag.Errorf("upload for %s failed, error: %v", m.name, err)
113114
}
114115

115116
return nil

‎bundle/artifacts/autodetect.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55

66
"github.com/databricks/cli/bundle"
77
"github.com/databricks/cli/bundle/artifacts/whl"
8+
"github.com/databricks/cli/libs/diag"
89
"github.com/databricks/cli/libs/log"
910
)
1011

@@ -19,7 +20,7 @@ func (m *autodetect) Name() string {
1920
return "artifacts.DetectPackages"
2021
}
2122

22-
func (m *autodetect) Apply(ctx context.Context, b *bundle.Bundle) error {
23+
func (m *autodetect) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
2324
// If artifacts section explicitly defined, do not try to auto detect packages
2425
if b.Config.Artifacts != nil {
2526
log.Debugf(ctx, "artifacts block is defined, skipping auto-detecting")

‎bundle/artifacts/build.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"path/filepath"
77

88
"github.com/databricks/cli/bundle"
9+
"github.com/databricks/cli/libs/diag"
910
)
1011

1112
func BuildAll() bundle.Mutator {
@@ -27,18 +28,18 @@ func (m *build) Name() string {
2728
return fmt.Sprintf("artifacts.Build(%s)", m.name)
2829
}
2930

30-
func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error {
31+
func (m *build) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
3132
artifact, ok := b.Config.Artifacts[m.name]
3233
if !ok {
33-
return fmt.Errorf("artifact doesn't exist: %s", m.name)
34+
return diag.Errorf("artifact doesn't exist: %s", m.name)
3435
}
3536

3637
// Skip building if build command is not specified or infered
3738
if artifact.BuildCommand == "" {
3839
// If no build command was specified or infered and there is no
3940
// artifact output files specified, artifact is misconfigured
4041
if len(artifact.Files) == 0 {
41-
return fmt.Errorf("misconfigured artifact: please specify 'build' or 'files' property")
42+
return diag.Errorf("misconfigured artifact: please specify 'build' or 'files' property")
4243
}
4344
return nil
4445
}

‎bundle/artifacts/infer.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/databricks/cli/bundle"
88
"github.com/databricks/cli/bundle/artifacts/whl"
99
"github.com/databricks/cli/bundle/config"
10+
"github.com/databricks/cli/libs/diag"
1011
)
1112

1213
var inferMutators map[config.ArtifactType]mutatorFactory = map[config.ArtifactType]mutatorFactory{
@@ -41,10 +42,10 @@ func (m *infer) Name() string {
4142
return fmt.Sprintf("artifacts.Infer(%s)", m.name)
4243
}
4344

44-
func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) error {
45+
func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
4546
artifact, ok := b.Config.Artifacts[m.name]
4647
if !ok {
47-
return fmt.Errorf("artifact doesn't exist: %s", m.name)
48+
return diag.Errorf("artifact doesn't exist: %s", m.name)
4849
}
4950

5051
// only try to infer command if it's not already defined

‎bundle/artifacts/upload.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/databricks/cli/bundle"
99
"github.com/databricks/cli/bundle/config"
10+
"github.com/databricks/cli/libs/diag"
1011
"github.com/databricks/databricks-sdk-go/service/workspace"
1112
)
1213

@@ -33,14 +34,14 @@ func (m *upload) Name() string {
3334
return fmt.Sprintf("artifacts.Upload(%s)", m.name)
3435
}
3536

36-
func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) error {
37+
func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
3738
artifact, ok := b.Config.Artifacts[m.name]
3839
if !ok {
39-
return fmt.Errorf("artifact doesn't exist: %s", m.name)
40+
return diag.Errorf("artifact doesn't exist: %s", m.name)
4041
}
4142

4243
if len(artifact.Files) == 0 {
43-
return fmt.Errorf("artifact source is not configured: %s", m.name)
44+
return diag.Errorf("artifact source is not configured: %s", m.name)
4445
}
4546

4647
// Check if source paths are absolute, if not, make them absolute
@@ -57,11 +58,11 @@ func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) error {
5758
for _, f := range artifact.Files {
5859
matches, err := filepath.Glob(f.Source)
5960
if err != nil {
60-
return fmt.Errorf("unable to find files for %s: %w", f.Source, err)
61+
return diag.Errorf("unable to find files for %s: %v", f.Source, err)
6162
}
6263

6364
if len(matches) == 0 {
64-
return fmt.Errorf("no files found for %s", f.Source)
65+
return diag.Errorf("no files found for %s", f.Source)
6566
}
6667

6768
for _, match := range matches {
@@ -81,10 +82,10 @@ func (m *cleanUp) Name() string {
8182
return "artifacts.CleanUp"
8283
}
8384

84-
func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) error {
85+
func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
8586
uploadPath, err := getUploadBasePath(b)
8687
if err != nil {
87-
return err
88+
return diag.FromErr(err)
8889
}
8990

9091
b.WorkspaceClient().Workspace.Delete(ctx, workspace.Delete{
@@ -94,7 +95,7 @@ func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) error {
9495

9596
err = b.WorkspaceClient().Workspace.MkdirsByPath(ctx, uploadPath)
9697
if err != nil {
97-
return fmt.Errorf("unable to create directory for %s: %w", uploadPath, err)
98+
return diag.Errorf("unable to create directory for %s: %v", uploadPath, err)
9899
}
99100

100101
return nil

‎bundle/artifacts/upload_test.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,14 @@ import (
99
"github.com/databricks/cli/bundle"
1010
"github.com/databricks/cli/bundle/config"
1111
"github.com/databricks/cli/bundle/internal/bundletest"
12+
"github.com/databricks/cli/libs/diag"
1213
"github.com/databricks/cli/libs/testfile"
1314
"github.com/stretchr/testify/require"
1415
)
1516

1617
type noop struct{}
1718

18-
func (n *noop) Apply(context.Context, *bundle.Bundle) error {
19+
func (n *noop) Apply(context.Context, *bundle.Bundle) diag.Diagnostics {
1920
return nil
2021
}
2122

@@ -57,8 +58,8 @@ func TestExpandGlobFilesSource(t *testing.T) {
5758
return &noop{}
5859
}
5960

60-
err = bundle.Apply(context.Background(), b, u)
61-
require.NoError(t, err)
61+
diags := bundle.Apply(context.Background(), b, u)
62+
require.NoError(t, diags.Error())
6263

6364
require.Equal(t, 2, len(b.Config.Artifacts["test"].Files))
6465
require.Equal(t, filepath.Join(rootPath, "test", "myjar1.jar"), b.Config.Artifacts["test"].Files[0].Source)
@@ -93,6 +94,6 @@ func TestExpandGlobFilesSourceWithNoMatches(t *testing.T) {
9394
return &noop{}
9495
}
9596

96-
err = bundle.Apply(context.Background(), b, u)
97-
require.ErrorContains(t, err, "no files found for")
97+
diags := bundle.Apply(context.Background(), b, u)
98+
require.ErrorContains(t, diags.Error(), "no files found for")
9899
}

‎bundle/artifacts/whl/autodetect.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/databricks/cli/bundle"
1212
"github.com/databricks/cli/bundle/config"
1313
"github.com/databricks/cli/bundle/libraries"
14+
"github.com/databricks/cli/libs/diag"
1415
"github.com/databricks/cli/libs/log"
1516
)
1617

@@ -25,7 +26,7 @@ func (m *detectPkg) Name() string {
2526
return "artifacts.whl.AutoDetect"
2627
}
2728

28-
func (m *detectPkg) Apply(ctx context.Context, b *bundle.Bundle) error {
29+
func (m *detectPkg) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
2930
wheelTasks := libraries.FindAllWheelTasksWithLocalLibraries(b)
3031
if len(wheelTasks) == 0 {
3132
log.Infof(ctx, "No local wheel tasks in databricks.yml config, skipping auto detect")
@@ -50,7 +51,7 @@ func (m *detectPkg) Apply(ctx context.Context, b *bundle.Bundle) error {
5051

5152
pkgPath, err := filepath.Abs(b.Config.Path)
5253
if err != nil {
53-
return err
54+
return diag.FromErr(err)
5455
}
5556
b.Config.Artifacts[module] = &config.Artifact{
5657
Path: pkgPath,

‎bundle/artifacts/whl/build.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/databricks/cli/bundle"
1010
"github.com/databricks/cli/bundle/config"
1111
"github.com/databricks/cli/libs/cmdio"
12+
"github.com/databricks/cli/libs/diag"
1213
"github.com/databricks/cli/libs/log"
1314
"github.com/databricks/cli/libs/python"
1415
)
@@ -27,10 +28,10 @@ func (m *build) Name() string {
2728
return fmt.Sprintf("artifacts.whl.Build(%s)", m.name)
2829
}
2930

30-
func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error {
31+
func (m *build) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
3132
artifact, ok := b.Config.Artifacts[m.name]
3233
if !ok {
33-
return fmt.Errorf("artifact doesn't exist: %s", m.name)
34+
return diag.Errorf("artifact doesn't exist: %s", m.name)
3435
}
3536

3637
cmdio.LogString(ctx, fmt.Sprintf("Building %s...", m.name))
@@ -43,13 +44,13 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error {
4344

4445
out, err := artifact.Build(ctx)
4546
if err != nil {
46-
return fmt.Errorf("build failed %s, error: %w, output: %s", m.name, err, out)
47+
return diag.Errorf("build failed %s, error: %v, output: %s", m.name, err, out)
4748
}
4849
log.Infof(ctx, "Build succeeded")
4950

5051
wheels := python.FindFilesWithSuffixInPath(distPath, ".whl")
5152
if len(wheels) == 0 {
52-
return fmt.Errorf("cannot find built wheel in %s for package %s", dir, m.name)
53+
return diag.Errorf("cannot find built wheel in %s for package %s", dir, m.name)
5354
}
5455
for _, wheel := range wheels {
5556
artifact.Files = append(artifact.Files, config.ArtifactFile{

‎bundle/artifacts/whl/from_libraries.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/databricks/cli/bundle"
88
"github.com/databricks/cli/bundle/config"
99
"github.com/databricks/cli/bundle/libraries"
10+
"github.com/databricks/cli/libs/diag"
1011
"github.com/databricks/cli/libs/log"
1112
)
1213

@@ -20,7 +21,7 @@ func (m *fromLibraries) Name() string {
2021
return "artifacts.whl.DefineArtifactsFromLibraries"
2122
}
2223

23-
func (*fromLibraries) Apply(ctx context.Context, b *bundle.Bundle) error {
24+
func (*fromLibraries) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
2425
if len(b.Config.Artifacts) != 0 {
2526
log.Debugf(ctx, "Skipping defining artifacts from libraries because artifacts section is explicitly defined")
2627
return nil

0 commit comments

Comments
 (0)