wip

Author: denik

acceptance/bundle/migrate/default-python/out.state_after_migration.json

@@ -49,7 +49,7 @@
       "job_cluster_key": "job_cluster",
       "libraries": [
        {
-        "whl": ".databricks/bundle/dev/patched_wheels/python_artifact_my_default_python/my_default_python-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl"
+        "whl": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl"
        }
       ],
       "notebook_task": {
@@ -66,7 +66,7 @@
       "job_cluster_key": "job_cluster",
       "libraries": [
        {
-        "whl": ".databricks/bundle/dev/patched_wheels/python_artifact_my_default_python/my_default_python-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl"
+        "whl": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][0]-py3-none-any.whl"
        }
       ],
       "python_wheel_task": {

acceptance/bundle/migrate/default-python/output.txt

@@ -38,12 +38,12 @@ To undo the migration, remove [TEST_TMP_DIR]/my_default_python/.databricks/bundl
 >>> jq .. | .libraries? | select(.) ../out.state_after_migration.json
 [
   {
-    "whl": ".databricks/bundle/dev/patched_wheels/python_artifact_my_default_python/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl"
+    "whl": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl"
   }
 ]
 [
   {
-    "whl": ".databricks/bundle/dev/patched_wheels/python_artifact_my_default_python/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl"
+    "whl": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS][1]-py3-none-any.whl"
   }
 ]
 [
@@ -74,22 +74,6 @@ Deployment complete!
 >>> print_state.py
 
 >>> diff.py ../out.state_after_migration.json ../out.state_after_deploy.json
---- ../out.state_after_migration.json
-+++ ../out.state_after_deploy.json
-@@ -50,5 +50,5 @@
-       "libraries": [
-        {
--        "whl": ".databricks/bundle/dev/patched_wheels/python_artifact_my_default_python/my_default_python-0.0.1+[UNIX_TIME_NANOS]-py3-none-any.whl"
-+        "whl": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS]-py3-none-any.whl"
-        }
-       ],
-@@ -67,5 +67,5 @@
-       "libraries": [
-        {
--        "whl": ".databricks/bundle/dev/patched_wheels/python_artifact_my_default_python/my_default_python-0.0.1+[UNIX_TIME_NANOS]-py3-none-any.whl"
-+        "whl": "/Workspace/Users/[USERNAME]/.bundle/my_default_python/dev/artifacts/.internal/my_default_python-0.0.1+[UNIX_TIME_NANOS]-py3-none-any.whl"
-        }
-       ],
 
 === Extra plan: should have no drift
 >>> DATABRICKS_BUNDLE_ENGINE= [CLI] bundle plan

bundle/phases/build.go

@@ -9,11 +9,15 @@ import (
 	"github.com/databricks/cli/bundle/config/mutator"
 	"github.com/databricks/cli/bundle/libraries"
 	"github.com/databricks/cli/bundle/scripts"
+	"github.com/databricks/cli/bundle/trampoline"
 	"github.com/databricks/cli/libs/log"
+	"github.com/databricks/cli/libs/logdiag"
 )
 
+type LibLocationMap map[string][]libraries.LocationToUpdate
+
 // The build phase builds artifacts.
-func Build(ctx context.Context, b *bundle.Bundle) {
+func Build(ctx context.Context, b *bundle.Bundle) LibLocationMap {
 	log.Info(ctx, "Phase: build")
 
 	bundle.ApplySeqContext(ctx, b,
@@ -36,4 +40,17 @@ func Build(ctx context.Context, b *bundle.Bundle) {
 		// SwitchToPatchedWheels must be run after ExpandGlobReferences and after build phase because it Artifact.Source and Artifact.Patched populated
 		libraries.SwitchToPatchedWheels(),
 	)
+
+	libs, diags := libraries.ReplaceWithRemotePath(ctx, b)
+	for _, diag := range diags {
+		logdiag.LogDiag(ctx, diag)
+	}
+
+	bundle.ApplyContext(ctx, b,
+		// TransformWheelTask must be run after ReplaceWithRemotePath so we can use correct remote path in the
+		// transformed notebook
+		trampoline.TransformWheelTask(),
+	)
+
+	return libs
 }

bundle/phases/deploy.go

@@ -136,7 +136,7 @@ func uploadLibraries(ctx context.Context, b *bundle.Bundle, libs map[string][]li
 }
 
 // The deploy phase deploys artifacts and resources.
-func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHandler, engine engine.EngineType) {
+func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHandler, engine engine.EngineType, libs map[string][]libraries.LocationToUpdate) {
 	log.Info(ctx, "Phase: deploy")
 
 	// Core mutators that CRUD resources and modify deployment state. These
@@ -156,11 +156,6 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand
 		bundle.ApplyContext(ctx, b, lock.Release(lock.GoalDeploy))
 	}()
 
-	libs := DeployPrepare(ctx, b, false, engine)
-	if logdiag.HasError(ctx) {
-		return
-	}
-
 	uploadLibraries(ctx, b, libs)
 	if logdiag.HasError(ctx) {
 		return

bundle/phases/plan.go

@@ -11,35 +11,19 @@ import (
 	"github.com/databricks/cli/bundle/deploy"
 	"github.com/databricks/cli/bundle/deploy/terraform"
 	"github.com/databricks/cli/bundle/deployplan"
-	"github.com/databricks/cli/bundle/libraries"
 	"github.com/databricks/cli/bundle/statemgmt"
-	"github.com/databricks/cli/bundle/trampoline"
 	"github.com/databricks/cli/libs/dyn"
-	"github.com/databricks/cli/libs/logdiag"
 )
 
-// DeployPrepare is common set of mutators between "bundle plan" and "bundle deploy".
-// This function does not make any mutations in the workspace remotely, only in-memory bundle config mutations
-func DeployPrepare(ctx context.Context, b *bundle.Bundle, isPlan bool, engine engine.EngineType) map[string][]libraries.LocationToUpdate {
+// PreDeployChecks is common set of mutators between "bundle plan" and "bundle deploy".
+// Note, it is not run in "bundle migrate" so it must not modify the config
+func PreDeployChecks(ctx context.Context, b *bundle.Bundle, isPlan bool, engine engine.EngineType) {
 	bundle.ApplySeqContext(ctx, b,
 		terraform.CheckDashboardsModifiedRemotely(isPlan, engine),
 		deploy.StatePull(),
 		mutator.ValidateGitDetails(),
 		statemgmt.CheckRunningResource(engine),
 	)
-
-	libs, diags := libraries.ReplaceWithRemotePath(ctx, b)
-	for _, diag := range diags {
-		logdiag.LogDiag(ctx, diag)
-	}
-
-	bundle.ApplySeqContext(ctx, b,
-		// TransformWheelTask must be run after ReplaceWithRemotePath so we can use correct remote path in the
-		// transformed notebook
-		trampoline.TransformWheelTask(),
-	)
-
-	return libs
 }
 
 // checkForPreventDestroy checks if the resource has lifecycle.prevent_destroy set, but the plan calls for this resource to be recreated or destroyed.

cmd/bundle/deploy.go

@@ -59,11 +59,12 @@ See https://docs.databricks.com/en/dev-tools/bundles/index.html for more informa
 					b.Config.Bundle.Deployment.FailOnActiveRuns = failOnActiveRuns
 				}
 			},
-			Verbose:      verbose,
-			AlwaysPull:   true,
-			FastValidate: true,
-			Build:        true,
-			Deploy:       true,
+			Verbose:         verbose,
+			AlwaysPull:      true,
+			FastValidate:    true,
+			Build:           true,
+			PreDeployChecks: true,
+			Deploy:          true,
 		})
 
 		return err

cmd/bundle/plan.go

@@ -35,10 +35,10 @@ It is useful for previewing changes before running 'bundle deploy'.`,
 
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
 		opts := utils.ProcessOptions{
-			AlwaysPull:    true,
-			FastValidate:  true,
-			Build:         true,
-			DeployPrepare: true,
+			AlwaysPull:      true,
+			FastValidate:    true,
+			Build:           true,
+			PreDeployChecks: true,
 		}
 
 		// Only add InitFunc if we need to set force or cluster ID

cmd/bundle/utils/process.go

@@ -59,11 +59,11 @@ type ProcessOptions struct {
 	SkipEngineEnvVar bool
 
 	// If true, call corresponding phase:
-	FastValidate  bool
-	Validate      bool
-	Build         bool
-	DeployPrepare bool
-	Deploy        bool
+	FastValidate    bool
+	Validate        bool
+	Build           bool
+	PreDeployChecks bool
+	Deploy          bool
 
 	// Indicate whether the bundle operation originates from the pipelines CLI
 	IsPipelinesCLI bool
@@ -152,7 +152,7 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (*bundle.Bundle,
 
 	var stateDesc *statemgmt.StateDesc
 
-	shouldReadState := opts.ReadState || opts.AlwaysPull || opts.InitIDs || opts.ErrorOnEmptyState || opts.DeployPrepare || opts.Deploy
+	shouldReadState := opts.ReadState || opts.AlwaysPull || opts.InitIDs || opts.ErrorOnEmptyState || opts.PreDeployChecks || opts.Deploy
 
 	if shouldReadState {
 		// PullResourcesState depends on stateFiler which needs b.Config.Workspace.StatePath which is set in phases.Initialize
@@ -206,9 +206,11 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (*bundle.Bundle,
 		}
 	}
 
-	if opts.Build {
+	var libs phases.LibLocationMap
+
+	if opts.Build || opts.Deploy {
 		t2 := time.Now()
-		phases.Build(ctx, b)
+		libs = phases.Build(ctx, b)
 		b.Metrics.ExecutionTimes = append(b.Metrics.ExecutionTimes, protos.IntMapEntry{
 			Key:   "phases.Build",
 			Value: time.Since(t2).Milliseconds(),
@@ -219,12 +221,13 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (*bundle.Bundle,
 		}
 	}
 
-	if opts.DeployPrepare {
-		if opts.Deploy {
-			panic("Deploy already calls DeployPrepare internally")
-		}
+	if opts.PreDeployChecks {
 		downgradeWarningToError := !opts.Deploy
-		_ = phases.DeployPrepare(ctx, b, downgradeWarningToError, stateDesc.Engine)
+		phases.PreDeployChecks(ctx, b, downgradeWarningToError, stateDesc.Engine)
+
+		if logdiag.HasError(ctx) {
+			return b, stateDesc, root.ErrAlreadyPrinted
+		}
 	}
 
 	if opts.Deploy {
@@ -236,7 +239,7 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (*bundle.Bundle,
 		}
 
 		t3 := time.Now()
-		phases.Deploy(ctx, b, outputHandler, stateDesc.Engine)
+		phases.Deploy(ctx, b, outputHandler, stateDesc.Engine, libs)
 		b.Metrics.ExecutionTimes = append(b.Metrics.ExecutionTimes, protos.IntMapEntry{
 			Key:   "phases.Deploy",
 			Value: time.Since(t3).Milliseconds(),