Skip to content

Commit

Permalink
Make bundle loaders return diagnostics (#1319)
Browse files Browse the repository at this point in the history
## Changes

The function signature of Cobra's `PreRunE` function has an `error`
return value. We'd like to start returning `diag.Diagnostics` after
loading a bundle, so this is incompatible. This change modifies all
usage of `PreRunE` to load a bundle to inline function calls in the
command's `RunE` function.

## Tests

* Unit tests pass.
* Integration tests pass.
  • Loading branch information
pietern authored Mar 28, 2024
1 parent 5df4c7e commit b21e3c8
Show file tree
Hide file tree
Showing 18 changed files with 305 additions and 199 deletions.
14 changes: 8 additions & 6 deletions cmd/bundle/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@ import (

func newDeployCommand() *cobra.Command {
cmd := &cobra.Command{
Use: "deploy",
Short: "Deploy bundle",
Args: root.NoArgs,
PreRunE: utils.ConfigureBundleWithVariables,
Use: "deploy",
Short: "Deploy bundle",
Args: root.NoArgs,
}

var force bool
Expand All @@ -30,7 +29,10 @@ func newDeployCommand() *cobra.Command {

cmd.RunE = func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
b := bundle.Get(ctx)
b, diags := utils.ConfigureBundleWithVariables(cmd)
if err := diags.Error(); err != nil {
return diags.Error()
}

bundle.ApplyFunc(ctx, b, func(context.Context, *bundle.Bundle) diag.Diagnostics {
b.Config.Bundle.Force = force
Expand All @@ -46,7 +48,7 @@ func newDeployCommand() *cobra.Command {
return nil
})

diags := bundle.Apply(ctx, b, bundle.Seq(
diags = bundle.Apply(ctx, b, bundle.Seq(
phases.Initialize(),
phases.Build(),
phases.Deploy(),
Expand Down
15 changes: 9 additions & 6 deletions cmd/bundle/deployment/bind.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ import (

func newBindCommand() *cobra.Command {
cmd := &cobra.Command{
Use: "bind KEY RESOURCE_ID",
Short: "Bind bundle-defined resources to existing resources",
Args: root.ExactArgs(2),
PreRunE: utils.ConfigureBundleWithVariables,
Use: "bind KEY RESOURCE_ID",
Short: "Bind bundle-defined resources to existing resources",
Args: root.ExactArgs(2),
}

var autoApprove bool
Expand All @@ -29,7 +28,11 @@ func newBindCommand() *cobra.Command {

cmd.RunE = func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
b := bundle.Get(ctx)
b, diags := utils.ConfigureBundleWithVariables(cmd)
if err := diags.Error(); err != nil {
return diags.Error()
}

resource, err := b.Config.Resources.FindResourceByConfigKey(args[0])
if err != nil {
return err
Expand All @@ -50,7 +53,7 @@ func newBindCommand() *cobra.Command {
return nil
})

diags := bundle.Apply(ctx, b, bundle.Seq(
diags = bundle.Apply(ctx, b, bundle.Seq(
phases.Initialize(),
phases.Bind(&terraform.BindOptions{
AutoApprove: autoApprove,
Expand Down
15 changes: 9 additions & 6 deletions cmd/bundle/deployment/unbind.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,21 @@ import (

func newUnbindCommand() *cobra.Command {
cmd := &cobra.Command{
Use: "unbind KEY",
Short: "Unbind bundle-defined resources from its managed remote resource",
Args: root.ExactArgs(1),
PreRunE: utils.ConfigureBundleWithVariables,
Use: "unbind KEY",
Short: "Unbind bundle-defined resources from its managed remote resource",
Args: root.ExactArgs(1),
}

var forceLock bool
cmd.Flags().BoolVar(&forceLock, "force-lock", false, "Force acquisition of deployment lock.")

cmd.RunE = func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
b := bundle.Get(ctx)
b, diags := utils.ConfigureBundleWithVariables(cmd)
if err := diags.Error(); err != nil {
return diags.Error()
}

resource, err := b.Config.Resources.FindResourceByConfigKey(args[0])
if err != nil {
return err
Expand All @@ -35,7 +38,7 @@ func newUnbindCommand() *cobra.Command {
return nil
})

diags := bundle.Apply(cmd.Context(), b, bundle.Seq(
diags = bundle.Apply(cmd.Context(), b, bundle.Seq(
phases.Initialize(),
phases.Unbind(resource.TerraformResourceName(), args[0]),
))
Expand Down
14 changes: 8 additions & 6 deletions cmd/bundle/destroy.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ import (

func newDestroyCommand() *cobra.Command {
cmd := &cobra.Command{
Use: "destroy",
Short: "Destroy deployed bundle resources",
Args: root.NoArgs,
PreRunE: utils.ConfigureBundleWithVariables,
Use: "destroy",
Short: "Destroy deployed bundle resources",
Args: root.NoArgs,
}

var autoApprove bool
Expand All @@ -31,7 +30,10 @@ func newDestroyCommand() *cobra.Command {

cmd.RunE = func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
b := bundle.Get(ctx)
b, diags := utils.ConfigureBundleWithVariables(cmd)
if err := diags.Error(); err != nil {
return diags.Error()
}

bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
// If `--force-lock` is specified, force acquisition of the deployment lock.
Expand All @@ -58,7 +60,7 @@ func newDestroyCommand() *cobra.Command {
return fmt.Errorf("please specify --auto-approve since selected logging format is json")
}

diags := bundle.Apply(ctx, b, bundle.Seq(
diags = bundle.Apply(ctx, b, bundle.Seq(
phases.Initialize(),
phases.Build(),
phases.Destroy(),
Expand Down
8 changes: 3 additions & 5 deletions cmd/bundle/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,16 @@ package bundle

import (
"github.com/databricks/cli/cmd/bundle/generate"
"github.com/databricks/cli/cmd/bundle/utils"
"github.com/spf13/cobra"
)

func newGenerateCommand() *cobra.Command {
var key string

cmd := &cobra.Command{
Use: "generate",
Short: "Generate bundle configuration",
Long: "Generate bundle configuration",
PreRunE: utils.ConfigureBundleWithVariables,
Use: "generate",
Short: "Generate bundle configuration",
Long: "Generate bundle configuration",
}

cmd.AddCommand(generate.NewGenerateJobCommand())
Expand Down
13 changes: 7 additions & 6 deletions cmd/bundle/generate/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"os"
"path/filepath"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config/generate"
"github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/libs/cmdio"
Expand All @@ -24,9 +23,8 @@ func NewGenerateJobCommand() *cobra.Command {
var force bool

cmd := &cobra.Command{
Use: "job",
Short: "Generate bundle configuration for a job",
PreRunE: root.MustConfigureBundle,
Use: "job",
Short: "Generate bundle configuration for a job",
}

cmd.Flags().Int64Var(&jobId, "existing-job-id", 0, `Job ID of the job to generate config for`)
Expand All @@ -43,9 +41,12 @@ func NewGenerateJobCommand() *cobra.Command {

cmd.RunE = func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
b := bundle.Get(ctx)
w := b.WorkspaceClient()
b, diags := root.MustConfigureBundle(cmd)
if err := diags.Error(); err != nil {
return diags.Error()
}

w := b.WorkspaceClient()
job, err := w.Jobs.Get(ctx, jobs.GetJobRequest{JobId: jobId})
if err != nil {
return err
Expand Down
13 changes: 7 additions & 6 deletions cmd/bundle/generate/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"os"
"path/filepath"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config/generate"
"github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/libs/cmdio"
Expand All @@ -24,9 +23,8 @@ func NewGeneratePipelineCommand() *cobra.Command {
var force bool

cmd := &cobra.Command{
Use: "pipeline",
Short: "Generate bundle configuration for a pipeline",
PreRunE: root.MustConfigureBundle,
Use: "pipeline",
Short: "Generate bundle configuration for a pipeline",
}

cmd.Flags().StringVar(&pipelineId, "existing-pipeline-id", "", `ID of the pipeline to generate config for`)
Expand All @@ -43,9 +41,12 @@ func NewGeneratePipelineCommand() *cobra.Command {

cmd.RunE = func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
b := bundle.Get(ctx)
w := b.WorkspaceClient()
b, diags := root.MustConfigureBundle(cmd)
if err := diags.Error(); err != nil {
return diags.Error()
}

w := b.WorkspaceClient()
pipeline, err := w.Pipelines.Get(ctx, pipelines.GetPipelineRequest{PipelineId: pipelineId})
if err != nil {
return err
Expand Down
2 changes: 0 additions & 2 deletions cmd/bundle/launch.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ func newLaunchCommand() *cobra.Command {

// We're not ready to expose this command until we specify its semantics.
Hidden: true,

PreRunE: root.MustConfigureBundle,
}

cmd.RunE = func(cmd *cobra.Command, args []string) error {
Expand Down
19 changes: 10 additions & 9 deletions cmd/bundle/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,9 @@ import (

func newRunCommand() *cobra.Command {
cmd := &cobra.Command{
Use: "run [flags] KEY",
Short: "Run a resource (e.g. a job or a pipeline)",
Args: root.MaximumNArgs(1),
PreRunE: utils.ConfigureBundleWithVariables,
Use: "run [flags] KEY",
Short: "Run a resource (e.g. a job or a pipeline)",
Args: root.MaximumNArgs(1),
}

var runOptions run.Options
Expand All @@ -33,9 +32,12 @@ func newRunCommand() *cobra.Command {

cmd.RunE = func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
b := bundle.Get(ctx)
b, diags := utils.ConfigureBundleWithVariables(cmd)
if err := diags.Error(); err != nil {
return diags.Error()
}

diags := bundle.Apply(ctx, b, bundle.Seq(
diags = bundle.Apply(ctx, b, bundle.Seq(
phases.Initialize(),
terraform.Interpolate(),
terraform.Write(),
Expand Down Expand Up @@ -109,15 +111,14 @@ func newRunCommand() *cobra.Command {
return nil, cobra.ShellCompDirectiveNoFileComp
}

err := root.MustConfigureBundle(cmd, args)
if err != nil {
b, diags := root.MustConfigureBundle(cmd)
if err := diags.Error(); err != nil {
cobra.CompErrorln(err.Error())
return nil, cobra.ShellCompDirectiveError
}

// No completion in the context of a bundle.
// Source and destination paths are taken from bundle configuration.
b := bundle.GetOrNil(cmd.Context())
if b == nil {
return nil, cobra.ShellCompDirectiveNoFileComp
}
Expand Down
21 changes: 12 additions & 9 deletions cmd/bundle/summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ import (

func newSummaryCommand() *cobra.Command {
cmd := &cobra.Command{
Use: "summary",
Short: "Describe the bundle resources and their deployment states",
Args: root.NoArgs,
PreRunE: utils.ConfigureBundleWithVariables,
Use: "summary",
Short: "Describe the bundle resources and their deployment states",
Args: root.NoArgs,

// This command is currently intended for the Databricks VSCode extension only
Hidden: true,
Expand All @@ -31,14 +30,18 @@ func newSummaryCommand() *cobra.Command {
cmd.Flags().BoolVar(&forcePull, "force-pull", false, "Skip local cache and load the state from the remote workspace")

cmd.RunE = func(cmd *cobra.Command, args []string) error {
b := bundle.Get(cmd.Context())
ctx := cmd.Context()
b, diags := utils.ConfigureBundleWithVariables(cmd)
if err := diags.Error(); err != nil {
return diags.Error()
}

diags := bundle.Apply(cmd.Context(), b, phases.Initialize())
diags = bundle.Apply(ctx, b, phases.Initialize())
if err := diags.Error(); err != nil {
return err
}

cacheDir, err := terraform.Dir(cmd.Context(), b)
cacheDir, err := terraform.Dir(ctx, b)
if err != nil {
return err
}
Expand All @@ -47,7 +50,7 @@ func newSummaryCommand() *cobra.Command {
noCache := errors.Is(stateFileErr, os.ErrNotExist) || errors.Is(configFileErr, os.ErrNotExist)

if forcePull || noCache {
diags = bundle.Apply(cmd.Context(), b, bundle.Seq(
diags = bundle.Apply(ctx, b, bundle.Seq(
terraform.StatePull(),
terraform.Interpolate(),
terraform.Write(),
Expand All @@ -57,7 +60,7 @@ func newSummaryCommand() *cobra.Command {
}
}

diags = bundle.Apply(cmd.Context(), b, terraform.Load())
diags = bundle.Apply(ctx, b, terraform.Load())
if err := diags.Error(); err != nil {
return err
}
Expand Down
11 changes: 6 additions & 5 deletions cmd/bundle/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ func newSyncCommand() *cobra.Command {
Use: "sync [flags]",
Short: "Synchronize bundle tree to the workspace",
Args: root.NoArgs,

PreRunE: utils.ConfigureBundleWithVariables,
}

var f syncFlags
Expand All @@ -46,10 +44,14 @@ func newSyncCommand() *cobra.Command {
cmd.Flags().BoolVar(&f.watch, "watch", false, "watch local file system for changes")

cmd.RunE = func(cmd *cobra.Command, args []string) error {
b := bundle.Get(cmd.Context())
ctx := cmd.Context()
b, diags := utils.ConfigureBundleWithVariables(cmd)
if err := diags.Error(); err != nil {
return diags.Error()
}

// Run initialize phase to make sure paths are set.
diags := bundle.Apply(cmd.Context(), b, phases.Initialize())
diags = bundle.Apply(ctx, b, phases.Initialize())
if err := diags.Error(); err != nil {
return err
}
Expand All @@ -59,7 +61,6 @@ func newSyncCommand() *cobra.Command {
return err
}

ctx := cmd.Context()
s, err := sync.New(ctx, *opts)
if err != nil {
return err
Expand Down
3 changes: 0 additions & 3 deletions cmd/bundle/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package bundle
import (
"fmt"

"github.com/databricks/cli/cmd/root"
"github.com/spf13/cobra"
)

Expand All @@ -15,8 +14,6 @@ func newTestCommand() *cobra.Command {

// We're not ready to expose this command until we specify its semantics.
Hidden: true,

PreRunE: root.MustConfigureBundle,
}

cmd.RunE = func(cmd *cobra.Command, args []string) error {
Expand Down
Loading

0 comments on commit b21e3c8

Please sign in to comment.