From 5bc5c3c26acf671026217b71bd52afb72cf3a472 Mon Sep 17 00:00:00 2001 From: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com> Date: Tue, 9 Jul 2024 20:38:38 +0530 Subject: [PATCH] Return early in bundle destroy if no deployment exists (#1581) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Changes This PR: 1. Moves the if mutator to the bundle package, to live with all-time greats such as `bundle.Seq` and `bundle.Defer`. Also adds unit tests. 2. `bundle destroy` now returns early if `root_path` does not exist. We do this by leveraging a `bundle.If` condition. ## Tests Unit tests and manually. Here's an example of what it'll look like once the bundle is destroyed. ``` ➜ bundle-playground git:(master) ✗ cli bundle destroy No active deployment found to destroy! ``` I would have added some e2e coverage for this as well, but the `cobraTestRunner.Run()` method does not seem to return stdout/stderr logs correctly. We can probably punt looking into it. --- bundle/config/mutator/if.go | 36 ------------------------- bundle/if.go | 40 ++++++++++++++++++++++++++++ bundle/if_test.go | 53 +++++++++++++++++++++++++++++++++++++ bundle/phases/destroy.go | 29 ++++++++++++++++++-- bundle/python/transform.go | 8 +++--- 5 files changed, 125 insertions(+), 41 deletions(-) delete mode 100644 bundle/config/mutator/if.go create mode 100644 bundle/if.go create mode 100644 bundle/if_test.go diff --git a/bundle/config/mutator/if.go b/bundle/config/mutator/if.go deleted file mode 100644 index 1b7856b3..00000000 --- a/bundle/config/mutator/if.go +++ /dev/null @@ -1,36 +0,0 @@ -package mutator - -import ( - "context" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/libs/diag" -) - -type ifMutator struct { - condition func(*bundle.Bundle) bool - onTrueMutator bundle.Mutator - onFalseMutator bundle.Mutator -} - -func If( - condition func(*bundle.Bundle) bool, - onTrueMutator bundle.Mutator, - onFalseMutator bundle.Mutator, -) bundle.Mutator { - return &ifMutator{ - condition, onTrueMutator, onFalseMutator, - } -} - -func (m *ifMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - if m.condition(b) { - return bundle.Apply(ctx, b, m.onTrueMutator) - } else { - return bundle.Apply(ctx, b, m.onFalseMutator) - } -} - -func (m *ifMutator) Name() string { - return "If" -} diff --git a/bundle/if.go b/bundle/if.go new file mode 100644 index 00000000..bad1d72d --- /dev/null +++ b/bundle/if.go @@ -0,0 +1,40 @@ +package bundle + +import ( + "context" + + "github.com/databricks/cli/libs/diag" +) + +type ifMutator struct { + condition func(context.Context, *Bundle) (bool, error) + onTrueMutator Mutator + onFalseMutator Mutator +} + +func If( + condition func(context.Context, *Bundle) (bool, error), + onTrueMutator Mutator, + onFalseMutator Mutator, +) Mutator { + return &ifMutator{ + condition, onTrueMutator, onFalseMutator, + } +} + +func (m *ifMutator) Apply(ctx context.Context, b *Bundle) diag.Diagnostics { + v, err := m.condition(ctx, b) + if err != nil { + return diag.FromErr(err) + } + + if v { + return Apply(ctx, b, m.onTrueMutator) + } else { + return Apply(ctx, b, m.onFalseMutator) + } +} + +func (m *ifMutator) Name() string { + return "If" +} diff --git a/bundle/if_test.go b/bundle/if_test.go new file mode 100644 index 00000000..b3fc0b9d --- /dev/null +++ b/bundle/if_test.go @@ -0,0 +1,53 @@ +package bundle + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIfMutatorTrue(t *testing.T) { + m1 := &testMutator{} + m2 := &testMutator{} + ifMutator := If(func(context.Context, *Bundle) (bool, error) { + return true, nil + }, m1, m2) + + b := &Bundle{} + diags := Apply(context.Background(), b, ifMutator) + assert.NoError(t, diags.Error()) + + assert.Equal(t, 1, m1.applyCalled) + assert.Equal(t, 0, m2.applyCalled) +} + +func TestIfMutatorFalse(t *testing.T) { + m1 := &testMutator{} + m2 := &testMutator{} + ifMutator := If(func(context.Context, *Bundle) (bool, error) { + return false, nil + }, m1, m2) + + b := &Bundle{} + diags := Apply(context.Background(), b, ifMutator) + assert.NoError(t, diags.Error()) + + assert.Equal(t, 0, m1.applyCalled) + assert.Equal(t, 1, m2.applyCalled) +} + +func TestIfMutatorError(t *testing.T) { + m1 := &testMutator{} + m2 := &testMutator{} + ifMutator := If(func(context.Context, *Bundle) (bool, error) { + return true, assert.AnError + }, m1, m2) + + b := &Bundle{} + diags := Apply(context.Background(), b, ifMutator) + assert.Error(t, diags.Error()) + + assert.Equal(t, 0, m1.applyCalled) + assert.Equal(t, 0, m2.applyCalled) +} diff --git a/bundle/phases/destroy.go b/bundle/phases/destroy.go index f974a056..f1beace8 100644 --- a/bundle/phases/destroy.go +++ b/bundle/phases/destroy.go @@ -1,15 +1,33 @@ package phases import ( + "context" + "errors" + "net/http" + "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy/files" "github.com/databricks/cli/bundle/deploy/lock" "github.com/databricks/cli/bundle/deploy/terraform" + "github.com/databricks/cli/libs/log" + "github.com/databricks/databricks-sdk-go/apierr" ) +func assertRootPathExists(ctx context.Context, b *bundle.Bundle) (bool, error) { + w := b.WorkspaceClient() + _, err := w.Workspace.GetStatusByPath(ctx, b.Config.Workspace.RootPath) + + var aerr *apierr.APIError + if errors.As(err, &aerr) && aerr.StatusCode == http.StatusNotFound { + log.Infof(ctx, "Root path does not exist: %s", b.Config.Workspace.RootPath) + return false, nil + } + + return true, err +} + // The destroy phase deletes artifacts and resources. func Destroy() bundle.Mutator { - destroyMutator := bundle.Seq( lock.Acquire(), bundle.Defer( @@ -29,6 +47,13 @@ func Destroy() bundle.Mutator { return newPhase( "destroy", - []bundle.Mutator{destroyMutator}, + []bundle.Mutator{ + // Only run deploy mutator if root path exists. + bundle.If( + assertRootPathExists, + destroyMutator, + bundle.LogString("No active deployment found to destroy!"), + ), + }, ) } diff --git a/bundle/python/transform.go b/bundle/python/transform.go index 457b45f7..9d3b1ab6 100644 --- a/bundle/python/transform.go +++ b/bundle/python/transform.go @@ -1,6 +1,7 @@ package python import ( + "context" "fmt" "strconv" "strings" @@ -63,9 +64,10 @@ dbutils.notebook.exit(s) // which installs uploaded wheels using %pip and then calling corresponding // entry point. func TransformWheelTask() bundle.Mutator { - return mutator.If( - func(b *bundle.Bundle) bool { - return b.Config.Experimental != nil && b.Config.Experimental.PythonWheelWrapper + return bundle.If( + func(_ context.Context, b *bundle.Bundle) (bool, error) { + res := b.Config.Experimental != nil && b.Config.Experimental.PythonWheelWrapper + return res, nil }, mutator.NewTrampoline( "python_wheel",