From 6b22fdebbaa98d7190a0d62f2a24e17d34ee7d0e Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Tue, 5 Nov 2024 18:12:45 +0100 Subject: [PATCH 01/30] Prototype for in-place deployments in DEV mode --- bundle/config/mutator/apply_presets.go | 10 +++++++++ bundle/config/mutator/process_target_mode.go | 5 +++++ bundle/config/mutator/translate_paths.go | 23 ++++++++++++++++++++ bundle/config/presets.go | 5 +++++ bundle/deploy/files/upload.go | 6 +++++ 5 files changed, 49 insertions(+) diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index d2a1d0c7..4b29f005 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/textutil" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/jobs" @@ -221,6 +222,15 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos dashboard.DisplayName = prefix + dashboard.DisplayName } + root := b.SyncRoot.Native() + _, ok := env.Lookup(ctx, envDatabricksRuntimeVersion) + isInWorkspace := ok && strings.HasPrefix(root, "/Workspace/") + + if !isInWorkspace { + disabled := false + b.Config.Presets.InPlaceDeployment = &disabled + } + return diags } diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index 44b53681..6ea0e7ef 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -57,6 +57,11 @@ func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) { t.TriggerPauseStatus = config.Paused } + if !config.IsExplicitlyDisabled(t.InPlaceDeployment) { + enabled := true + t.InPlaceDeployment = &enabled + } + if !config.IsExplicitlyDisabled(t.PipelinesDevelopment) { enabled := true t.PipelinesDevelopment = &enabled diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index 82b0b3ca..fe3f2f8b 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -11,6 +11,8 @@ import ( "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/libraries" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/notebook" @@ -129,6 +131,9 @@ func (t *translateContext) translateNotebookPath(literal, localFullPath, localRe return "", ErrIsNotNotebook{localFullPath} } + if t.shouldTranslateRemotePaths(localFullPath) { + return localFullPath, nil + } // Upon import, notebooks are stripped of their extension. return strings.TrimSuffix(remotePath, filepath.Ext(localFullPath)), nil } @@ -144,6 +149,9 @@ func (t *translateContext) translateFilePath(literal, localFullPath, localRelPat if nb { return "", ErrIsNotebook{localFullPath} } + if t.shouldTranslateRemotePaths(localFullPath) { + return localFullPath, nil + } return remotePath, nil } @@ -155,10 +163,16 @@ func (t *translateContext) translateDirectoryPath(literal, localFullPath, localR if !info.IsDir() { return "", fmt.Errorf("%s is not a directory", localFullPath) } + if t.shouldTranslateRemotePaths(localFullPath) { + return localFullPath, nil + } return remotePath, nil } func (t *translateContext) translateNoOp(literal, localFullPath, localRelPath, remotePath string) (string, error) { + if t.shouldTranslateRemotePaths(localFullPath) { + return localFullPath, nil + } return localRelPath, nil } @@ -177,6 +191,10 @@ func (t *translateContext) retainLocalAbsoluteFilePath(literal, localFullPath, l } func (t *translateContext) translateNoOpWithPrefix(literal, localFullPath, localRelPath, remotePath string) (string, error) { + if t.shouldTranslateRemotePaths(localFullPath) { + return localFullPath, nil + } + if !strings.HasPrefix(localRelPath, ".") { localRelPath = "." + string(filepath.Separator) + localRelPath } @@ -217,6 +235,11 @@ func (t *translateContext) rewriteRelativeTo(p dyn.Path, v dyn.Value, fn rewrite return dyn.InvalidValue, err } +func (t *translateContext) shouldTranslateRemotePaths(localFullPath string) bool { + return config.IsExplicitlyEnabled(t.b.Config.Presets.InPlaceDeployment) && + libraries.IsWorkspacePath(localFullPath) +} + func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { t := &translateContext{ b: b, diff --git a/bundle/config/presets.go b/bundle/config/presets.go index 61009a25..c5fb6e10 100644 --- a/bundle/config/presets.go +++ b/bundle/config/presets.go @@ -17,6 +17,11 @@ type Presets struct { // JobsMaxConcurrentRuns is the default value for the max concurrent runs of jobs. JobsMaxConcurrentRuns int `json:"jobs_max_concurrent_runs,omitempty"` + // InPlaceDeployment indicates whether in-place deployment is enabled. Works only in workspace + // When set to true, resources created during deployment will point to source files in the workspace instead of their workspace copies. + // No resources will be uploaded to workspace + InPlaceDeployment *bool `json:"in_place_deployment,omitempty"` + // Tags to add to all resources. Tags map[string]string `json:"tags,omitempty"` } diff --git a/bundle/deploy/files/upload.go b/bundle/deploy/files/upload.go index bab4e176..bdad6f01 100644 --- a/bundle/deploy/files/upload.go +++ b/bundle/deploy/files/upload.go @@ -7,6 +7,7 @@ import ( "io/fs" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/permissions" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/diag" @@ -23,6 +24,11 @@ func (m *upload) Name() string { } func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + if config.IsExplicitlyEnabled(b.Config.Presets.InPlaceDeployment) { + cmdio.LogString(ctx, "Bundle files uploading skipped: in-place deployment is enabled") + return nil + } + cmdio.LogString(ctx, fmt.Sprintf("Uploading bundle files to %s...", b.Config.Workspace.FilePath)) opts, err := GetSyncOptions(ctx, bundle.ReadOnly(b)) if err != nil { From fbb9be9bdc27e5763f0f509d666d47a265206c21 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Tue, 5 Nov 2024 19:03:27 +0100 Subject: [PATCH 02/30] fix: Add SyncRoot mock to tests --- bundle/config/mutator/apply_presets_test.go | 7 +++++++ bundle/config/mutator/process_target_mode_test.go | 2 ++ 2 files changed, 9 insertions(+) diff --git a/bundle/config/mutator/apply_presets_test.go b/bundle/config/mutator/apply_presets_test.go index 24295da4..52efb07b 100644 --- a/bundle/config/mutator/apply_presets_test.go +++ b/bundle/config/mutator/apply_presets_test.go @@ -8,6 +8,7 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/vfs" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/stretchr/testify/require" @@ -55,6 +56,7 @@ func TestApplyPresetsPrefix(t *testing.T) { NamePrefix: tt.prefix, }, }, + SyncRoot: vfs.MustNew(t.TempDir()), } ctx := context.Background() @@ -101,6 +103,7 @@ func TestApplyPresetsPrefixForUcSchema(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { b := &bundle.Bundle{ + SyncRoot: vfs.MustNew(t.TempDir()), Config: config.Root{ Resources: config.Resources{ Schemas: map[string]*resources.Schema{ @@ -180,6 +183,7 @@ func TestApplyPresetsTags(t *testing.T) { Tags: tt.tags, }, }, + SyncRoot: vfs.MustNew(t.TempDir()), } ctx := context.Background() @@ -239,6 +243,7 @@ func TestApplyPresetsJobsMaxConcurrentRuns(t *testing.T) { JobsMaxConcurrentRuns: tt.setting, }, }, + SyncRoot: vfs.MustNew(t.TempDir()), } ctx := context.Background() diag := bundle.Apply(ctx, b, mutator.ApplyPresets()) @@ -264,6 +269,7 @@ func TestApplyPresetsPrefixWithoutJobSettings(t *testing.T) { NamePrefix: "prefix-", }, }, + SyncRoot: vfs.MustNew(t.TempDir()), } ctx := context.Background() @@ -355,6 +361,7 @@ func TestApplyPresetsResourceNotDefined(t *testing.T) { TriggerPauseStatus: config.Paused, }, }, + SyncRoot: vfs.MustNew(t.TempDir()), } ctx := context.Background() diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index d76d2d8f..80a1e0bc 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/tags" + "github.com/databricks/cli/libs/vfs" sdkconfig "github.com/databricks/databricks-sdk-go/config" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/compute" @@ -133,6 +134,7 @@ func mockBundle(mode config.Mode) *bundle.Bundle { }, }, }, + SyncRoot: vfs.MustNew("/Users/lennart.kats@databricks.com"), // Use AWS implementation for testing. Tagging: tags.ForCloud(&sdkconfig.Config{ Host: "https://company.cloud.databricks.com", From a43f7ddad08b084f8a27eed65833da3f13f46a33 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Mon, 11 Nov 2024 19:36:31 +0100 Subject: [PATCH 03/30] feat: Use global FilePath instead of locally overriden paths --- .../config/mutator/default_workspace_paths.go | 5 ++++ bundle/config/mutator/translate_paths.go | 23 ------------------- 2 files changed, 5 insertions(+), 23 deletions(-) diff --git a/bundle/config/mutator/default_workspace_paths.go b/bundle/config/mutator/default_workspace_paths.go index 02a1ddb3..9dfbf763 100644 --- a/bundle/config/mutator/default_workspace_paths.go +++ b/bundle/config/mutator/default_workspace_paths.go @@ -5,6 +5,7 @@ import ( "path" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/diag" ) @@ -25,6 +26,10 @@ func (m *defineDefaultWorkspacePaths) Apply(ctx context.Context, b *bundle.Bundl return diag.Errorf("unable to define default workspace paths: workspace root not defined") } + if config.IsExplicitlyEnabled((b.Config.Presets.InPlaceDeployment)) { + b.Config.Workspace.FilePath = b.BundleRootPath + } + if b.Config.Workspace.FilePath == "" { b.Config.Workspace.FilePath = path.Join(root, "files") } diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index b6fb1d3e..321fa5b3 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -11,8 +11,6 @@ import ( "strings" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/libraries" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/notebook" @@ -131,9 +129,6 @@ func (t *translateContext) translateNotebookPath(literal, localFullPath, localRe return "", ErrIsNotNotebook{localFullPath} } - if t.shouldTranslateRemotePaths(localFullPath) { - return localFullPath, nil - } // Upon import, notebooks are stripped of their extension. return strings.TrimSuffix(remotePath, filepath.Ext(localFullPath)), nil } @@ -149,9 +144,6 @@ func (t *translateContext) translateFilePath(literal, localFullPath, localRelPat if nb { return "", ErrIsNotebook{localFullPath} } - if t.shouldTranslateRemotePaths(localFullPath) { - return localFullPath, nil - } return remotePath, nil } @@ -163,16 +155,10 @@ func (t *translateContext) translateDirectoryPath(literal, localFullPath, localR if !info.IsDir() { return "", fmt.Errorf("%s is not a directory", localFullPath) } - if t.shouldTranslateRemotePaths(localFullPath) { - return localFullPath, nil - } return remotePath, nil } func (t *translateContext) translateNoOp(literal, localFullPath, localRelPath, remotePath string) (string, error) { - if t.shouldTranslateRemotePaths(localFullPath) { - return localFullPath, nil - } return localRelPath, nil } @@ -191,10 +177,6 @@ func (t *translateContext) retainLocalAbsoluteFilePath(literal, localFullPath, l } func (t *translateContext) translateNoOpWithPrefix(literal, localFullPath, localRelPath, remotePath string) (string, error) { - if t.shouldTranslateRemotePaths(localFullPath) { - return localFullPath, nil - } - if !strings.HasPrefix(localRelPath, ".") { localRelPath = "." + string(filepath.Separator) + localRelPath } @@ -235,11 +217,6 @@ func (t *translateContext) rewriteRelativeTo(p dyn.Path, v dyn.Value, fn rewrite return dyn.InvalidValue, err } -func (t *translateContext) shouldTranslateRemotePaths(localFullPath string) bool { - return config.IsExplicitlyEnabled(t.b.Config.Presets.InPlaceDeployment) && - libraries.IsWorkspacePath(localFullPath) -} - func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { t := &translateContext{ b: b, From 5a22151580540727a9b451014955db3b31d9d23a Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Tue, 12 Nov 2024 09:58:36 +0100 Subject: [PATCH 04/30] feat: Move FilePath update to apply preset step --- bundle/config/mutator/apply_presets.go | 16 ++++++++++------ bundle/config/mutator/default_workspace_paths.go | 5 ----- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index 4b29f005..cdccf549 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -222,13 +222,17 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos dashboard.DisplayName = prefix + dashboard.DisplayName } - root := b.SyncRoot.Native() - _, ok := env.Lookup(ctx, envDatabricksRuntimeVersion) - isInWorkspace := ok && strings.HasPrefix(root, "/Workspace/") + if config.IsExplicitlyEnabled((b.Config.Presets.InPlaceDeployment)) { + root := b.SyncRoot.Native() + _, ok := env.Lookup(ctx, envDatabricksRuntimeVersion) + isInWorkspace := ok && strings.HasPrefix(root, "/Workspace/") - if !isInWorkspace { - disabled := false - b.Config.Presets.InPlaceDeployment = &disabled + if isInWorkspace { + b.Config.Workspace.FilePath = b.BundleRootPath + } else { + disabled := false + b.Config.Presets.InPlaceDeployment = &disabled + } } return diags diff --git a/bundle/config/mutator/default_workspace_paths.go b/bundle/config/mutator/default_workspace_paths.go index 9dfbf763..02a1ddb3 100644 --- a/bundle/config/mutator/default_workspace_paths.go +++ b/bundle/config/mutator/default_workspace_paths.go @@ -5,7 +5,6 @@ import ( "path" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/diag" ) @@ -26,10 +25,6 @@ func (m *defineDefaultWorkspacePaths) Apply(ctx context.Context, b *bundle.Bundl return diag.Errorf("unable to define default workspace paths: workspace root not defined") } - if config.IsExplicitlyEnabled((b.Config.Presets.InPlaceDeployment)) { - b.Config.Workspace.FilePath = b.BundleRootPath - } - if b.Config.Workspace.FilePath == "" { b.Config.Workspace.FilePath = path.Join(root, "files") } From b2164c0725b4972d72d0875c39fd3198a57824b8 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Tue, 12 Nov 2024 15:16:47 +0100 Subject: [PATCH 05/30] fix: Use SyncRootPath to allow using parent directories --- bundle/config/mutator/apply_presets.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index cdccf549..8dc012ea 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -228,7 +228,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos isInWorkspace := ok && strings.HasPrefix(root, "/Workspace/") if isInWorkspace { - b.Config.Workspace.FilePath = b.BundleRootPath + b.Config.Workspace.FilePath = b.SyncRootPath } else { disabled := false b.Config.Presets.InPlaceDeployment = &disabled From 26f24538eeda35ce8b65c278ef06f95fad4c4640 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Thu, 14 Nov 2024 18:18:17 +0100 Subject: [PATCH 06/30] feat: Use dbr package for runtime check --- bundle/config/mutator/apply_presets.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index 63780091..ca0869b5 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -223,11 +223,8 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos } if config.IsExplicitlyEnabled((b.Config.Presets.InPlaceDeployment)) { - root := b.SyncRoot.Native() - _, ok := env.Lookup(ctx, envDatabricksRuntimeVersion) isInWorkspace := ok && strings.HasPrefix(root, "/Workspace/") - - if isInWorkspace { + if isInWorkspace && dbr.RunsOnRuntime(ctx) { b.Config.Workspace.FilePath = b.SyncRootPath } else { disabled := false From 51c8ef592cd02a2071e7cdaeadff4c06064237eb Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Thu, 14 Nov 2024 18:38:31 +0100 Subject: [PATCH 07/30] fix: Applies autoformat --- bundle/config/mutator/apply_presets.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index ca0869b5..aad5504c 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -9,9 +9,9 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/dbr" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" - "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/textutil" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/jobs" From 4cf692924c62d69578dd16de2e5bd7da883b2565 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Thu, 14 Nov 2024 18:46:10 +0100 Subject: [PATCH 08/30] fix: Missing root variable --- bundle/config/mutator/apply_presets.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index aad5504c..2a9b0220 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -223,7 +223,8 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos } if config.IsExplicitlyEnabled((b.Config.Presets.InPlaceDeployment)) { - isInWorkspace := ok && strings.HasPrefix(root, "/Workspace/") + root := b.SyncRoot.Native() + isInWorkspace := strings.HasPrefix(root, "/Workspace/") if isInWorkspace && dbr.RunsOnRuntime(ctx) { b.Config.Workspace.FilePath = b.SyncRootPath } else { From 8cd95eb0ea0a83ca7ad0b313f1ea3d8ef868c931 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Fri, 15 Nov 2024 12:09:53 +0100 Subject: [PATCH 09/30] test: Apply presets unit test --- bundle/config/mutator/apply_presets_test.go | 84 +++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/bundle/config/mutator/apply_presets_test.go b/bundle/config/mutator/apply_presets_test.go index 52efb07b..65ac9129 100644 --- a/bundle/config/mutator/apply_presets_test.go +++ b/bundle/config/mutator/apply_presets_test.go @@ -8,6 +8,7 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/dbr" "github.com/databricks/cli/libs/vfs" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/jobs" @@ -371,3 +372,86 @@ func TestApplyPresetsResourceNotDefined(t *testing.T) { }) } } + +func TestApplyPresetsInPlaceDeployment(t *testing.T) { + testContext := context.TODO() + enabled := true + disabled := false + remotePath := "/Users/files" + workspacePath := "/Workspace/user.name@company.com" + + tests := []struct { + bundlePath string + ctx context.Context + name string + initialValue *bool + expectedValue *bool + expectedFilePath string + }{ + { + name: "preset enabled, bundle in Workspace, databricks runtime", + bundlePath: workspacePath, + ctx: dbr.MockRuntime(testContext, true), + initialValue: &enabled, + expectedValue: &enabled, + expectedFilePath: workspacePath, + }, + { + name: "preset enabled, bundle not in Workspace, databricks runtime", + bundlePath: "/Users/user.name@company.com", + ctx: dbr.MockRuntime(testContext, true), + initialValue: &enabled, + expectedValue: &disabled, + expectedFilePath: remotePath, + }, + { + name: "preset enabled, bundle in Workspace, not databricks runtime", + bundlePath: workspacePath, + ctx: dbr.MockRuntime(testContext, false), + initialValue: &enabled, + expectedValue: &disabled, + expectedFilePath: remotePath, + }, + { + name: "preset disabled, bundle in Workspace, databricks runtime", + bundlePath: workspacePath, + ctx: dbr.MockRuntime(testContext, true), + initialValue: &disabled, + expectedValue: &disabled, + expectedFilePath: remotePath, + }, + { + name: "preset nil, bundle in Workspace, databricks runtime", + bundlePath: workspacePath, + ctx: dbr.MockRuntime(testContext, true), + initialValue: nil, + expectedValue: nil, + expectedFilePath: remotePath, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Presets: config.Presets{ + InPlaceDeployment: tt.initialValue, + }, + Workspace: config.Workspace{ + FilePath: remotePath, + }, + }, + SyncRoot: vfs.MustNew(tt.bundlePath), + SyncRootPath: tt.bundlePath, + } + + diags := bundle.Apply(tt.ctx, b, mutator.ApplyPresets()) + if diags.HasError() { + t.Fatalf("unexpected error: %v", diags) + } + + require.Equal(t, tt.expectedFilePath, b.Config.Workspace.FilePath) + require.Equal(t, tt.expectedValue, b.Config.Presets.InPlaceDeployment) + }) + } +} From e9b72895dc0569bbd7aeff81e4a1bef792b729bf Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Fri, 15 Nov 2024 12:32:44 +0100 Subject: [PATCH 10/30] test: Process target mode --- bundle/config/mutator/apply_presets_test.go | 2 +- .../mutator/process_target_mode_test.go | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/bundle/config/mutator/apply_presets_test.go b/bundle/config/mutator/apply_presets_test.go index 65ac9129..9c4dcddd 100644 --- a/bundle/config/mutator/apply_presets_test.go +++ b/bundle/config/mutator/apply_presets_test.go @@ -374,7 +374,7 @@ func TestApplyPresetsResourceNotDefined(t *testing.T) { } func TestApplyPresetsInPlaceDeployment(t *testing.T) { - testContext := context.TODO() + testContext := context.Background() enabled := true disabled := false remotePath := "/Users/files" diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index 36cea812..81a2208a 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -9,6 +9,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/dbr" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/tags" "github.com/databricks/cli/libs/vfs" @@ -517,3 +518,31 @@ func TestPipelinesDevelopmentDisabled(t *testing.T) { assert.False(t, b.Config.Resources.Pipelines["pipeline1"].PipelineSpec.Development) } + +func TestInPlaceDeploymentEnabled(t *testing.T) { + b, diags := processInPlaceBundle(true) + require.NoError(t, diags.Error()) + assert.True(t, *b.Config.Presets.InPlaceDeployment) + assert.Equal(t, b.Config.Workspace.FilePath, b.SyncRootPath) +} + +func TestInPlaceDeploymentDisabled(t *testing.T) { + b, diags := processInPlaceBundle(false) + require.NoError(t, diags.Error()) + assert.False(t, *b.Config.Presets.InPlaceDeployment) + assert.NotEqual(t, b.Config.Workspace.FilePath, b.SyncRootPath) +} + +func processInPlaceBundle(presetEnabled bool) (*bundle.Bundle, diag.Diagnostics) { + b := mockBundle(config.Development) + + workspacePath := "/Workspace/lennart@company.com/" + b.SyncRoot = vfs.MustNew(workspacePath) + b.SyncRootPath = workspacePath + b.Config.Presets.InPlaceDeployment = &presetEnabled + + ctx := dbr.MockRuntime(context.Background(), true) + m := bundle.Seq(ProcessTargetMode(), ApplyPresets()) + diags := bundle.Apply(ctx, b, m) + return b, diags +} From a3c6d571c8a70983eaa327f30490eeaa2fb4311a Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Mon, 18 Nov 2024 11:11:44 +0100 Subject: [PATCH 11/30] fix: Use SyncRootPath --- bundle/config/mutator/apply_presets.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index 2a9b0220..c9491b1a 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -223,7 +223,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos } if config.IsExplicitlyEnabled((b.Config.Presets.InPlaceDeployment)) { - root := b.SyncRoot.Native() + root := b.SyncRootPath isInWorkspace := strings.HasPrefix(root, "/Workspace/") if isInWorkspace && dbr.RunsOnRuntime(ctx) { b.Config.Workspace.FilePath = b.SyncRootPath From 80ea3a05e30fb9fa3019394bf9e36ec8fff3249f Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Mon, 18 Nov 2024 11:24:33 +0100 Subject: [PATCH 12/30] fix: Move SyncRoot field to the top og the struct --- bundle/config/mutator/apply_presets_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/bundle/config/mutator/apply_presets_test.go b/bundle/config/mutator/apply_presets_test.go index 9c4dcddd..662b18a1 100644 --- a/bundle/config/mutator/apply_presets_test.go +++ b/bundle/config/mutator/apply_presets_test.go @@ -47,6 +47,7 @@ func TestApplyPresetsPrefix(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { b := &bundle.Bundle{ + SyncRoot: vfs.MustNew(t.TempDir()), Config: config.Root{ Resources: config.Resources{ Jobs: map[string]*resources.Job{ @@ -57,7 +58,6 @@ func TestApplyPresetsPrefix(t *testing.T) { NamePrefix: tt.prefix, }, }, - SyncRoot: vfs.MustNew(t.TempDir()), } ctx := context.Background() @@ -174,6 +174,7 @@ func TestApplyPresetsTags(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { b := &bundle.Bundle{ + SyncRoot: vfs.MustNew(t.TempDir()), Config: config.Root{ Resources: config.Resources{ Jobs: map[string]*resources.Job{ @@ -184,7 +185,6 @@ func TestApplyPresetsTags(t *testing.T) { Tags: tt.tags, }, }, - SyncRoot: vfs.MustNew(t.TempDir()), } ctx := context.Background() @@ -234,6 +234,7 @@ func TestApplyPresetsJobsMaxConcurrentRuns(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { b := &bundle.Bundle{ + SyncRoot: vfs.MustNew(t.TempDir()), Config: config.Root{ Resources: config.Resources{ Jobs: map[string]*resources.Job{ @@ -244,7 +245,6 @@ func TestApplyPresetsJobsMaxConcurrentRuns(t *testing.T) { JobsMaxConcurrentRuns: tt.setting, }, }, - SyncRoot: vfs.MustNew(t.TempDir()), } ctx := context.Background() diag := bundle.Apply(ctx, b, mutator.ApplyPresets()) @@ -260,6 +260,7 @@ func TestApplyPresetsJobsMaxConcurrentRuns(t *testing.T) { func TestApplyPresetsPrefixWithoutJobSettings(t *testing.T) { b := &bundle.Bundle{ + SyncRoot: vfs.MustNew(t.TempDir()), Config: config.Root{ Resources: config.Resources{ Jobs: map[string]*resources.Job{ @@ -270,7 +271,6 @@ func TestApplyPresetsPrefixWithoutJobSettings(t *testing.T) { NamePrefix: "prefix-", }, }, - SyncRoot: vfs.MustNew(t.TempDir()), } ctx := context.Background() @@ -356,13 +356,13 @@ func TestApplyPresetsResourceNotDefined(t *testing.T) { for _, tt := range tests { t.Run(tt.error, func(t *testing.T) { b := &bundle.Bundle{ + SyncRoot: vfs.MustNew(t.TempDir()), Config: config.Root{ Resources: tt.resources, Presets: config.Presets{ TriggerPauseStatus: config.Paused, }, }, - SyncRoot: vfs.MustNew(t.TempDir()), } ctx := context.Background() @@ -433,6 +433,8 @@ func TestApplyPresetsInPlaceDeployment(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { b := &bundle.Bundle{ + SyncRoot: vfs.MustNew(tt.bundlePath), + SyncRootPath: tt.bundlePath, Config: config.Root{ Presets: config.Presets{ InPlaceDeployment: tt.initialValue, @@ -441,8 +443,6 @@ func TestApplyPresetsInPlaceDeployment(t *testing.T) { FilePath: remotePath, }, }, - SyncRoot: vfs.MustNew(tt.bundlePath), - SyncRootPath: tt.bundlePath, } diags := bundle.Apply(tt.ctx, b, mutator.ApplyPresets()) From e7165ec17a7855adf12aafb581dbf7ea5288886f Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Mon, 18 Nov 2024 11:58:56 +0100 Subject: [PATCH 13/30] feat: Add Databricks Workspace conditions to dev mode setting. Add warning if conditions not met when preset is enabled by user --- bundle/config/mutator/apply_presets.go | 1 + bundle/config/mutator/apply_presets_test.go | 9 +++++++++ bundle/config/mutator/process_target_mode.go | 10 ++++++++-- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index c9491b1a..1e86da0a 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -230,6 +230,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos } else { disabled := false b.Config.Presets.InPlaceDeployment = &disabled + diags = diags.Extend(diag.Warningf("in-place deployment is available only in the Databricks Workspace")) } } diff --git a/bundle/config/mutator/apply_presets_test.go b/bundle/config/mutator/apply_presets_test.go index 662b18a1..eaf4a56c 100644 --- a/bundle/config/mutator/apply_presets_test.go +++ b/bundle/config/mutator/apply_presets_test.go @@ -387,6 +387,7 @@ func TestApplyPresetsInPlaceDeployment(t *testing.T) { initialValue *bool expectedValue *bool expectedFilePath string + expectedWarning string }{ { name: "preset enabled, bundle in Workspace, databricks runtime", @@ -395,6 +396,7 @@ func TestApplyPresetsInPlaceDeployment(t *testing.T) { initialValue: &enabled, expectedValue: &enabled, expectedFilePath: workspacePath, + expectedWarning: "", }, { name: "preset enabled, bundle not in Workspace, databricks runtime", @@ -403,6 +405,7 @@ func TestApplyPresetsInPlaceDeployment(t *testing.T) { initialValue: &enabled, expectedValue: &disabled, expectedFilePath: remotePath, + expectedWarning: "in-place deployment is available only in the Databricks Workspace", }, { name: "preset enabled, bundle in Workspace, not databricks runtime", @@ -411,6 +414,7 @@ func TestApplyPresetsInPlaceDeployment(t *testing.T) { initialValue: &enabled, expectedValue: &disabled, expectedFilePath: remotePath, + expectedWarning: "in-place deployment is available only in the Databricks Workspace", }, { name: "preset disabled, bundle in Workspace, databricks runtime", @@ -450,8 +454,13 @@ func TestApplyPresetsInPlaceDeployment(t *testing.T) { t.Fatalf("unexpected error: %v", diags) } + if tt.expectedWarning != "" { + require.Equal(t, tt.expectedWarning, diags[0].Summary) + } + require.Equal(t, tt.expectedFilePath, b.Config.Workspace.FilePath) require.Equal(t, tt.expectedValue, b.Config.Presets.InPlaceDeployment) }) } + } diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index 6ea0e7ef..81330d87 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -6,6 +6,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/dbr" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/iamutil" @@ -63,8 +64,13 @@ func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) { } if !config.IsExplicitlyDisabled(t.PipelinesDevelopment) { - enabled := true - t.PipelinesDevelopment = &enabled + root := b.SyncRootPath + isInWorkspace := strings.HasPrefix(root, "/Workspace/") + + if isInWorkspace && dbr.RunsOnRuntime(ctx) { + enabled := true + t.PipelinesDevelopment = &enabled + } } } From 49f6bc93543326ce7620629b374e45b87ace9a03 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Mon, 18 Nov 2024 12:08:10 +0100 Subject: [PATCH 14/30] fix: Skipping in-place tests on Windows --- bundle/config/mutator/apply_presets_test.go | 5 +++++ bundle/config/mutator/process_target_mode_test.go | 11 ++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/bundle/config/mutator/apply_presets_test.go b/bundle/config/mutator/apply_presets_test.go index eaf4a56c..5107faa5 100644 --- a/bundle/config/mutator/apply_presets_test.go +++ b/bundle/config/mutator/apply_presets_test.go @@ -2,6 +2,7 @@ package mutator_test import ( "context" + "runtime" "testing" "github.com/databricks/cli/bundle" @@ -374,6 +375,10 @@ func TestApplyPresetsResourceNotDefined(t *testing.T) { } func TestApplyPresetsInPlaceDeployment(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("this test is not applicable on Windows because in-place works only in the Databricks Workspace") + } + testContext := context.Background() enabled := true disabled := false diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index 81a2208a..e848e9f2 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -3,6 +3,7 @@ package mutator import ( "context" "reflect" + "runtime" "strings" "testing" @@ -520,20 +521,24 @@ func TestPipelinesDevelopmentDisabled(t *testing.T) { } func TestInPlaceDeploymentEnabled(t *testing.T) { - b, diags := processInPlaceBundle(true) + b, diags := processInPlaceBundle(t, true) require.NoError(t, diags.Error()) assert.True(t, *b.Config.Presets.InPlaceDeployment) assert.Equal(t, b.Config.Workspace.FilePath, b.SyncRootPath) } func TestInPlaceDeploymentDisabled(t *testing.T) { - b, diags := processInPlaceBundle(false) + b, diags := processInPlaceBundle(t, false) require.NoError(t, diags.Error()) assert.False(t, *b.Config.Presets.InPlaceDeployment) assert.NotEqual(t, b.Config.Workspace.FilePath, b.SyncRootPath) } -func processInPlaceBundle(presetEnabled bool) (*bundle.Bundle, diag.Diagnostics) { +func processInPlaceBundle(t *testing.T, presetEnabled bool) (*bundle.Bundle, diag.Diagnostics) { + if runtime.GOOS == "windows" { + t.Skip("this test is not applicable on Windows because in-place works only in the Databricks Workspace") + } + b := mockBundle(config.Development) workspacePath := "/Workspace/lennart@company.com/" From e65b50cf3cce36e06291e4b2ebe48a2014c7bd7d Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Mon, 18 Nov 2024 12:22:14 +0100 Subject: [PATCH 15/30] fix: Wrong condition --- bundle/config/mutator/process_target_mode.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index 81330d87..16b81ebb 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -59,18 +59,17 @@ func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) { } if !config.IsExplicitlyDisabled(t.InPlaceDeployment) { - enabled := true - t.InPlaceDeployment = &enabled + root := b.SyncRootPath + isInWorkspace := strings.HasPrefix(root, "/Workspace/") + if isInWorkspace && dbr.RunsOnRuntime(ctx) { + enabled := true + t.InPlaceDeployment = &enabled + } } if !config.IsExplicitlyDisabled(t.PipelinesDevelopment) { - root := b.SyncRootPath - isInWorkspace := strings.HasPrefix(root, "/Workspace/") - - if isInWorkspace && dbr.RunsOnRuntime(ctx) { - enabled := true - t.PipelinesDevelopment = &enabled - } + enabled := true + t.PipelinesDevelopment = &enabled } } From 53e1f6df6a2d809cc930568e29c9593b71d0942a Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Mon, 18 Nov 2024 16:05:21 +0100 Subject: [PATCH 16/30] fix: Skip permissions set and check for in-place --- bundle/bundle_read_only.go | 4 ++++ bundle/config/validate/folder_permissions.go | 4 ++++ bundle/config/validate/folder_permissions_test.go | 3 +++ bundle/permissions/workspace_root.go | 4 ++++ bundle/permissions/workspace_root_test.go | 2 ++ 5 files changed, 17 insertions(+) diff --git a/bundle/bundle_read_only.go b/bundle/bundle_read_only.go index ceab95c0..466b0903 100644 --- a/bundle/bundle_read_only.go +++ b/bundle/bundle_read_only.go @@ -32,6 +32,10 @@ func (r ReadOnlyBundle) SyncRoot() vfs.Path { return r.b.SyncRoot } +func (r ReadOnlyBundle) SyncRootPath() string { + return r.b.SyncRootPath +} + func (r ReadOnlyBundle) WorkspaceClient() *databricks.WorkspaceClient { return r.b.WorkspaceClient() } diff --git a/bundle/config/validate/folder_permissions.go b/bundle/config/validate/folder_permissions.go index 505e82a1..4cc2d9a0 100644 --- a/bundle/config/validate/folder_permissions.go +++ b/bundle/config/validate/folder_permissions.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "path" + "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/libraries" @@ -30,6 +31,9 @@ func (f *folderPermissions) Apply(ctx context.Context, b bundle.ReadOnlyBundle) g, ctx := errgroup.WithContext(ctx) results := make([]diag.Diagnostics, len(bundlePaths)) for i, p := range bundlePaths { + if strings.HasPrefix(p, b.SyncRootPath()) { + continue + } g.Go(func() error { results[i] = checkFolderPermission(ctx, b, p) return nil diff --git a/bundle/config/validate/folder_permissions_test.go b/bundle/config/validate/folder_permissions_test.go index 8e68c9fb..2af2a349 100644 --- a/bundle/config/validate/folder_permissions_test.go +++ b/bundle/config/validate/folder_permissions_test.go @@ -77,6 +77,7 @@ func TestFolderPermissionsInheritedWhenRootPathDoesNotExist(t *testing.T) { func TestValidateFolderPermissionsFailsOnMissingBundlePermission(t *testing.T) { b := &bundle.Bundle{ + SyncRootPath: t.TempDir(), Config: config.Root{ Workspace: config.Workspace{ RootPath: "/Workspace/Users/foo@bar.com", @@ -129,6 +130,7 @@ func TestValidateFolderPermissionsFailsOnMissingBundlePermission(t *testing.T) { func TestValidateFolderPermissionsFailsOnPermissionMismatch(t *testing.T) { b := &bundle.Bundle{ + SyncRootPath: t.TempDir(), Config: config.Root{ Workspace: config.Workspace{ RootPath: "/Workspace/Users/foo@bar.com", @@ -174,6 +176,7 @@ func TestValidateFolderPermissionsFailsOnPermissionMismatch(t *testing.T) { func TestValidateFolderPermissionsFailsOnNoRootFolder(t *testing.T) { b := &bundle.Bundle{ + SyncRootPath: t.TempDir(), Config: config.Root{ Workspace: config.Workspace{ RootPath: "/NotExisting", diff --git a/bundle/permissions/workspace_root.go b/bundle/permissions/workspace_root.go index de4f3a7f..665be88c 100644 --- a/bundle/permissions/workspace_root.go +++ b/bundle/permissions/workspace_root.go @@ -3,6 +3,7 @@ package permissions import ( "context" "fmt" + "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/libraries" @@ -59,6 +60,9 @@ func giveAccessForWorkspaceRoot(ctx context.Context, b *bundle.Bundle) error { g, ctx := errgroup.WithContext(ctx) for _, p := range bundlePaths { + if strings.HasPrefix(p, b.SyncRootPath) { + continue + } g.Go(func() error { return setPermissions(ctx, w, p, permissions) }) diff --git a/bundle/permissions/workspace_root_test.go b/bundle/permissions/workspace_root_test.go index c48704a6..7b993760 100644 --- a/bundle/permissions/workspace_root_test.go +++ b/bundle/permissions/workspace_root_test.go @@ -19,6 +19,7 @@ import ( func TestApplyWorkspaceRootPermissions(t *testing.T) { b := &bundle.Bundle{ + SyncRootPath: t.TempDir(), Config: config.Root{ Workspace: config.Workspace{ RootPath: "/Users/foo@bar.com", @@ -79,6 +80,7 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) { func TestApplyWorkspaceRootPermissionsForAllPaths(t *testing.T) { b := &bundle.Bundle{ + SyncRootPath: t.TempDir(), Config: config.Root{ Workspace: config.Workspace{ RootPath: "/Some/Root/Path", From 00bb683be91ff4b42ae72765164439e0f27cabe7 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Mon, 18 Nov 2024 16:13:14 +0100 Subject: [PATCH 17/30] fix: Remove unnecessary fields in apply presets test --- bundle/config/mutator/apply_presets_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/bundle/config/mutator/apply_presets_test.go b/bundle/config/mutator/apply_presets_test.go index 5107faa5..f70831ab 100644 --- a/bundle/config/mutator/apply_presets_test.go +++ b/bundle/config/mutator/apply_presets_test.go @@ -105,7 +105,6 @@ func TestApplyPresetsPrefixForUcSchema(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { b := &bundle.Bundle{ - SyncRoot: vfs.MustNew(t.TempDir()), Config: config.Root{ Resources: config.Resources{ Schemas: map[string]*resources.Schema{ @@ -175,7 +174,6 @@ func TestApplyPresetsTags(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { b := &bundle.Bundle{ - SyncRoot: vfs.MustNew(t.TempDir()), Config: config.Root{ Resources: config.Resources{ Jobs: map[string]*resources.Job{ @@ -235,7 +233,6 @@ func TestApplyPresetsJobsMaxConcurrentRuns(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { b := &bundle.Bundle{ - SyncRoot: vfs.MustNew(t.TempDir()), Config: config.Root{ Resources: config.Resources{ Jobs: map[string]*resources.Job{ @@ -261,7 +258,6 @@ func TestApplyPresetsJobsMaxConcurrentRuns(t *testing.T) { func TestApplyPresetsPrefixWithoutJobSettings(t *testing.T) { b := &bundle.Bundle{ - SyncRoot: vfs.MustNew(t.TempDir()), Config: config.Root{ Resources: config.Resources{ Jobs: map[string]*resources.Job{ @@ -357,7 +353,6 @@ func TestApplyPresetsResourceNotDefined(t *testing.T) { for _, tt := range tests { t.Run(tt.error, func(t *testing.T) { b := &bundle.Bundle{ - SyncRoot: vfs.MustNew(t.TempDir()), Config: config.Root{ Resources: tt.resources, Presets: config.Presets{ From e8825d559f63a38828b508f9cb8198162744ad2e Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Mon, 18 Nov 2024 16:19:26 +0100 Subject: [PATCH 18/30] fix: Reuse existing `root` variable --- bundle/config/mutator/apply_presets.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index 1e86da0a..e41bdf9c 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -226,7 +226,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos root := b.SyncRootPath isInWorkspace := strings.HasPrefix(root, "/Workspace/") if isInWorkspace && dbr.RunsOnRuntime(ctx) { - b.Config.Workspace.FilePath = b.SyncRootPath + b.Config.Workspace.FilePath = root } else { disabled := false b.Config.Presets.InPlaceDeployment = &disabled From 1d7b27e0ef4bb34ced32d7e9149390edce849bd5 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Mon, 18 Nov 2024 17:21:33 +0100 Subject: [PATCH 19/30] feat: Rename "in-place" to "source-linked" --- bundle/config/mutator/apply_presets.go | 6 +++--- bundle/config/mutator/apply_presets_test.go | 12 ++++++------ bundle/config/mutator/process_target_mode.go | 4 ++-- .../config/mutator/process_target_mode_test.go | 18 +++++++++--------- bundle/config/presets.go | 4 ++-- bundle/deploy/files/upload.go | 4 ++-- 6 files changed, 24 insertions(+), 24 deletions(-) diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index e41bdf9c..a376e303 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -222,15 +222,15 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos dashboard.DisplayName = prefix + dashboard.DisplayName } - if config.IsExplicitlyEnabled((b.Config.Presets.InPlaceDeployment)) { + if config.IsExplicitlyEnabled((b.Config.Presets.SourceLinkedDeployment)) { root := b.SyncRootPath isInWorkspace := strings.HasPrefix(root, "/Workspace/") if isInWorkspace && dbr.RunsOnRuntime(ctx) { b.Config.Workspace.FilePath = root } else { disabled := false - b.Config.Presets.InPlaceDeployment = &disabled - diags = diags.Extend(diag.Warningf("in-place deployment is available only in the Databricks Workspace")) + b.Config.Presets.SourceLinkedDeployment = &disabled + diags = diags.Extend(diag.Warningf("source-linked deployment is available only in the Databricks Workspace")) } } diff --git a/bundle/config/mutator/apply_presets_test.go b/bundle/config/mutator/apply_presets_test.go index f70831ab..8aa2addd 100644 --- a/bundle/config/mutator/apply_presets_test.go +++ b/bundle/config/mutator/apply_presets_test.go @@ -369,9 +369,9 @@ func TestApplyPresetsResourceNotDefined(t *testing.T) { } } -func TestApplyPresetsInPlaceDeployment(t *testing.T) { +func TestApplyPresetsSourceLinkedDeployment(t *testing.T) { if runtime.GOOS == "windows" { - t.Skip("this test is not applicable on Windows because in-place works only in the Databricks Workspace") + t.Skip("this test is not applicable on Windows because source-linked mode works only in the Databricks Workspace") } testContext := context.Background() @@ -405,7 +405,7 @@ func TestApplyPresetsInPlaceDeployment(t *testing.T) { initialValue: &enabled, expectedValue: &disabled, expectedFilePath: remotePath, - expectedWarning: "in-place deployment is available only in the Databricks Workspace", + expectedWarning: "source-linked deployment is available only in the Databricks Workspace", }, { name: "preset enabled, bundle in Workspace, not databricks runtime", @@ -414,7 +414,7 @@ func TestApplyPresetsInPlaceDeployment(t *testing.T) { initialValue: &enabled, expectedValue: &disabled, expectedFilePath: remotePath, - expectedWarning: "in-place deployment is available only in the Databricks Workspace", + expectedWarning: "source-linked deployment is available only in the Databricks Workspace", }, { name: "preset disabled, bundle in Workspace, databricks runtime", @@ -441,7 +441,7 @@ func TestApplyPresetsInPlaceDeployment(t *testing.T) { SyncRootPath: tt.bundlePath, Config: config.Root{ Presets: config.Presets{ - InPlaceDeployment: tt.initialValue, + SourceLinkedDeployment: tt.initialValue, }, Workspace: config.Workspace{ FilePath: remotePath, @@ -459,7 +459,7 @@ func TestApplyPresetsInPlaceDeployment(t *testing.T) { } require.Equal(t, tt.expectedFilePath, b.Config.Workspace.FilePath) - require.Equal(t, tt.expectedValue, b.Config.Presets.InPlaceDeployment) + require.Equal(t, tt.expectedValue, b.Config.Presets.SourceLinkedDeployment) }) } diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index 16b81ebb..77bb54cb 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -58,12 +58,12 @@ func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) { t.TriggerPauseStatus = config.Paused } - if !config.IsExplicitlyDisabled(t.InPlaceDeployment) { + if !config.IsExplicitlyDisabled(t.SourceLinkedDeployment) { root := b.SyncRootPath isInWorkspace := strings.HasPrefix(root, "/Workspace/") if isInWorkspace && dbr.RunsOnRuntime(ctx) { enabled := true - t.InPlaceDeployment = &enabled + t.SourceLinkedDeployment = &enabled } } diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index e848e9f2..d6985bda 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -520,23 +520,23 @@ func TestPipelinesDevelopmentDisabled(t *testing.T) { assert.False(t, b.Config.Resources.Pipelines["pipeline1"].PipelineSpec.Development) } -func TestInPlaceDeploymentEnabled(t *testing.T) { - b, diags := processInPlaceBundle(t, true) +func TestSourceLinkedDeploymentEnabled(t *testing.T) { + b, diags := processSourceLinkedBundle(t, true) require.NoError(t, diags.Error()) - assert.True(t, *b.Config.Presets.InPlaceDeployment) + assert.True(t, *b.Config.Presets.SourceLinkedDeployment) assert.Equal(t, b.Config.Workspace.FilePath, b.SyncRootPath) } -func TestInPlaceDeploymentDisabled(t *testing.T) { - b, diags := processInPlaceBundle(t, false) +func TestSourceLinkedDeploymentDisabled(t *testing.T) { + b, diags := processSourceLinkedBundle(t, false) require.NoError(t, diags.Error()) - assert.False(t, *b.Config.Presets.InPlaceDeployment) + assert.False(t, *b.Config.Presets.SourceLinkedDeployment) assert.NotEqual(t, b.Config.Workspace.FilePath, b.SyncRootPath) } -func processInPlaceBundle(t *testing.T, presetEnabled bool) (*bundle.Bundle, diag.Diagnostics) { +func processSourceLinkedBundle(t *testing.T, presetEnabled bool) (*bundle.Bundle, diag.Diagnostics) { if runtime.GOOS == "windows" { - t.Skip("this test is not applicable on Windows because in-place works only in the Databricks Workspace") + t.Skip("this test is not applicable on Windows because source-linked mode works only in the Databricks Workspace") } b := mockBundle(config.Development) @@ -544,7 +544,7 @@ func processInPlaceBundle(t *testing.T, presetEnabled bool) (*bundle.Bundle, dia workspacePath := "/Workspace/lennart@company.com/" b.SyncRoot = vfs.MustNew(workspacePath) b.SyncRootPath = workspacePath - b.Config.Presets.InPlaceDeployment = &presetEnabled + b.Config.Presets.SourceLinkedDeployment = &presetEnabled ctx := dbr.MockRuntime(context.Background(), true) m := bundle.Seq(ProcessTargetMode(), ApplyPresets()) diff --git a/bundle/config/presets.go b/bundle/config/presets.go index c5fb6e10..14c0d8a2 100644 --- a/bundle/config/presets.go +++ b/bundle/config/presets.go @@ -17,10 +17,10 @@ type Presets struct { // JobsMaxConcurrentRuns is the default value for the max concurrent runs of jobs. JobsMaxConcurrentRuns int `json:"jobs_max_concurrent_runs,omitempty"` - // InPlaceDeployment indicates whether in-place deployment is enabled. Works only in workspace + // SourceLinkedDeployment indicates whether source-linked deployment is enabled. Works only in Databricks Workspace // When set to true, resources created during deployment will point to source files in the workspace instead of their workspace copies. // No resources will be uploaded to workspace - InPlaceDeployment *bool `json:"in_place_deployment,omitempty"` + SourceLinkedDeployment *bool `json:"source_linked_deployment,omitempty"` // Tags to add to all resources. Tags map[string]string `json:"tags,omitempty"` diff --git a/bundle/deploy/files/upload.go b/bundle/deploy/files/upload.go index bdad6f01..f4b2b3f4 100644 --- a/bundle/deploy/files/upload.go +++ b/bundle/deploy/files/upload.go @@ -24,8 +24,8 @@ func (m *upload) Name() string { } func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - if config.IsExplicitlyEnabled(b.Config.Presets.InPlaceDeployment) { - cmdio.LogString(ctx, "Bundle files uploading skipped: in-place deployment is enabled") + if config.IsExplicitlyEnabled(b.Config.Presets.SourceLinkedDeployment) { + cmdio.LogString(ctx, "Source-linked deployment is enabled. Resources will point to the source files directly without making copies") return nil } From 55f715dddf91a977616c528c682f0b083d9eea7d Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Mon, 18 Nov 2024 17:31:52 +0100 Subject: [PATCH 20/30] fix: Broken test --- bundle/config/validate/folder_permissions_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/bundle/config/validate/folder_permissions_test.go b/bundle/config/validate/folder_permissions_test.go index 2af2a349..92ff0a0b 100644 --- a/bundle/config/validate/folder_permissions_test.go +++ b/bundle/config/validate/folder_permissions_test.go @@ -18,6 +18,7 @@ import ( func TestFolderPermissionsInheritedWhenRootPathDoesNotExist(t *testing.T) { b := &bundle.Bundle{ + SyncRootPath: t.TempDir(), Config: config.Root{ Workspace: config.Workspace{ RootPath: "/Workspace/Users/foo@bar.com", From 518aa14b6445a1963e08d073e9a68b525116cef0 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Mon, 18 Nov 2024 22:32:36 +0100 Subject: [PATCH 21/30] Revert "fix: Skip permissions set and check for in-place" This reverts commit 53e1f6df6a2d809cc930568e29c9593b71d0942a. --- bundle/bundle_read_only.go | 4 ---- bundle/config/validate/folder_permissions.go | 4 ---- bundle/config/validate/folder_permissions_test.go | 3 --- bundle/permissions/workspace_root.go | 4 ---- bundle/permissions/workspace_root_test.go | 2 -- 5 files changed, 17 deletions(-) diff --git a/bundle/bundle_read_only.go b/bundle/bundle_read_only.go index 466b0903..ceab95c0 100644 --- a/bundle/bundle_read_only.go +++ b/bundle/bundle_read_only.go @@ -32,10 +32,6 @@ func (r ReadOnlyBundle) SyncRoot() vfs.Path { return r.b.SyncRoot } -func (r ReadOnlyBundle) SyncRootPath() string { - return r.b.SyncRootPath -} - func (r ReadOnlyBundle) WorkspaceClient() *databricks.WorkspaceClient { return r.b.WorkspaceClient() } diff --git a/bundle/config/validate/folder_permissions.go b/bundle/config/validate/folder_permissions.go index 4cc2d9a0..505e82a1 100644 --- a/bundle/config/validate/folder_permissions.go +++ b/bundle/config/validate/folder_permissions.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "path" - "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/libraries" @@ -31,9 +30,6 @@ func (f *folderPermissions) Apply(ctx context.Context, b bundle.ReadOnlyBundle) g, ctx := errgroup.WithContext(ctx) results := make([]diag.Diagnostics, len(bundlePaths)) for i, p := range bundlePaths { - if strings.HasPrefix(p, b.SyncRootPath()) { - continue - } g.Go(func() error { results[i] = checkFolderPermission(ctx, b, p) return nil diff --git a/bundle/config/validate/folder_permissions_test.go b/bundle/config/validate/folder_permissions_test.go index 92ff0a0b..f609724e 100644 --- a/bundle/config/validate/folder_permissions_test.go +++ b/bundle/config/validate/folder_permissions_test.go @@ -78,7 +78,6 @@ func TestFolderPermissionsInheritedWhenRootPathDoesNotExist(t *testing.T) { func TestValidateFolderPermissionsFailsOnMissingBundlePermission(t *testing.T) { b := &bundle.Bundle{ - SyncRootPath: t.TempDir(), Config: config.Root{ Workspace: config.Workspace{ RootPath: "/Workspace/Users/foo@bar.com", @@ -131,7 +130,6 @@ func TestValidateFolderPermissionsFailsOnMissingBundlePermission(t *testing.T) { func TestValidateFolderPermissionsFailsOnPermissionMismatch(t *testing.T) { b := &bundle.Bundle{ - SyncRootPath: t.TempDir(), Config: config.Root{ Workspace: config.Workspace{ RootPath: "/Workspace/Users/foo@bar.com", @@ -177,7 +175,6 @@ func TestValidateFolderPermissionsFailsOnPermissionMismatch(t *testing.T) { func TestValidateFolderPermissionsFailsOnNoRootFolder(t *testing.T) { b := &bundle.Bundle{ - SyncRootPath: t.TempDir(), Config: config.Root{ Workspace: config.Workspace{ RootPath: "/NotExisting", diff --git a/bundle/permissions/workspace_root.go b/bundle/permissions/workspace_root.go index 665be88c..de4f3a7f 100644 --- a/bundle/permissions/workspace_root.go +++ b/bundle/permissions/workspace_root.go @@ -3,7 +3,6 @@ package permissions import ( "context" "fmt" - "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/libraries" @@ -60,9 +59,6 @@ func giveAccessForWorkspaceRoot(ctx context.Context, b *bundle.Bundle) error { g, ctx := errgroup.WithContext(ctx) for _, p := range bundlePaths { - if strings.HasPrefix(p, b.SyncRootPath) { - continue - } g.Go(func() error { return setPermissions(ctx, w, p, permissions) }) diff --git a/bundle/permissions/workspace_root_test.go b/bundle/permissions/workspace_root_test.go index 7b993760..c48704a6 100644 --- a/bundle/permissions/workspace_root_test.go +++ b/bundle/permissions/workspace_root_test.go @@ -19,7 +19,6 @@ import ( func TestApplyWorkspaceRootPermissions(t *testing.T) { b := &bundle.Bundle{ - SyncRootPath: t.TempDir(), Config: config.Root{ Workspace: config.Workspace{ RootPath: "/Users/foo@bar.com", @@ -80,7 +79,6 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) { func TestApplyWorkspaceRootPermissionsForAllPaths(t *testing.T) { b := &bundle.Bundle{ - SyncRootPath: t.TempDir(), Config: config.Root{ Workspace: config.Workspace{ RootPath: "/Some/Root/Path", From aeb9813d255f5b3d6cc179975105783630031d3e Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Tue, 19 Nov 2024 10:40:16 +0100 Subject: [PATCH 22/30] feat: Use path translations instead of overriding config --- bundle/config/mutator/apply_presets.go | 7 +- bundle/config/mutator/apply_presets_test.go | 79 ++++++++----------- .../mutator/process_target_mode_test.go | 3 - bundle/config/mutator/translate_paths.go | 10 ++- 4 files changed, 43 insertions(+), 56 deletions(-) diff --git a/bundle/config/mutator/apply_presets.go b/bundle/config/mutator/apply_presets.go index a376e303..9cec704e 100644 --- a/bundle/config/mutator/apply_presets.go +++ b/bundle/config/mutator/apply_presets.go @@ -223,11 +223,8 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos } if config.IsExplicitlyEnabled((b.Config.Presets.SourceLinkedDeployment)) { - root := b.SyncRootPath - isInWorkspace := strings.HasPrefix(root, "/Workspace/") - if isInWorkspace && dbr.RunsOnRuntime(ctx) { - b.Config.Workspace.FilePath = root - } else { + isDatabricksWorkspace := dbr.RunsOnRuntime(ctx) && strings.HasPrefix(b.SyncRootPath, "/Workspace/") + if !isDatabricksWorkspace { disabled := false b.Config.Presets.SourceLinkedDeployment = &disabled diags = diags.Extend(diag.Warningf("source-linked deployment is available only in the Databricks Workspace")) diff --git a/bundle/config/mutator/apply_presets_test.go b/bundle/config/mutator/apply_presets_test.go index 8aa2addd..dc56c27f 100644 --- a/bundle/config/mutator/apply_presets_test.go +++ b/bundle/config/mutator/apply_presets_test.go @@ -377,75 +377,63 @@ func TestApplyPresetsSourceLinkedDeployment(t *testing.T) { testContext := context.Background() enabled := true disabled := false - remotePath := "/Users/files" workspacePath := "/Workspace/user.name@company.com" tests := []struct { - bundlePath string - ctx context.Context - name string - initialValue *bool - expectedValue *bool - expectedFilePath string - expectedWarning string + bundlePath string + ctx context.Context + name string + initialValue *bool + expectedValue *bool + expectedWarning string }{ { - name: "preset enabled, bundle in Workspace, databricks runtime", - bundlePath: workspacePath, - ctx: dbr.MockRuntime(testContext, true), - initialValue: &enabled, - expectedValue: &enabled, - expectedFilePath: workspacePath, - expectedWarning: "", + name: "preset enabled, bundle in Workspace, databricks runtime", + bundlePath: workspacePath, + ctx: dbr.MockRuntime(testContext, true), + initialValue: &enabled, + expectedValue: &enabled, }, { - name: "preset enabled, bundle not in Workspace, databricks runtime", - bundlePath: "/Users/user.name@company.com", - ctx: dbr.MockRuntime(testContext, true), - initialValue: &enabled, - expectedValue: &disabled, - expectedFilePath: remotePath, - expectedWarning: "source-linked deployment is available only in the Databricks Workspace", + name: "preset enabled, bundle not in Workspace, databricks runtime", + bundlePath: "/Users/user.name@company.com", + ctx: dbr.MockRuntime(testContext, true), + initialValue: &enabled, + expectedValue: &disabled, + expectedWarning: "source-linked deployment is available only in the Databricks Workspace", }, { - name: "preset enabled, bundle in Workspace, not databricks runtime", - bundlePath: workspacePath, - ctx: dbr.MockRuntime(testContext, false), - initialValue: &enabled, - expectedValue: &disabled, - expectedFilePath: remotePath, - expectedWarning: "source-linked deployment is available only in the Databricks Workspace", + name: "preset enabled, bundle in Workspace, not databricks runtime", + bundlePath: workspacePath, + ctx: dbr.MockRuntime(testContext, false), + initialValue: &enabled, + expectedValue: &disabled, + expectedWarning: "source-linked deployment is available only in the Databricks Workspace", }, { - name: "preset disabled, bundle in Workspace, databricks runtime", - bundlePath: workspacePath, - ctx: dbr.MockRuntime(testContext, true), - initialValue: &disabled, - expectedValue: &disabled, - expectedFilePath: remotePath, + name: "preset disabled, bundle in Workspace, databricks runtime", + bundlePath: workspacePath, + ctx: dbr.MockRuntime(testContext, true), + initialValue: &disabled, + expectedValue: &disabled, }, { - name: "preset nil, bundle in Workspace, databricks runtime", - bundlePath: workspacePath, - ctx: dbr.MockRuntime(testContext, true), - initialValue: nil, - expectedValue: nil, - expectedFilePath: remotePath, + name: "preset nil, bundle in Workspace, databricks runtime", + bundlePath: workspacePath, + ctx: dbr.MockRuntime(testContext, true), + initialValue: nil, + expectedValue: nil, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { b := &bundle.Bundle{ - SyncRoot: vfs.MustNew(tt.bundlePath), SyncRootPath: tt.bundlePath, Config: config.Root{ Presets: config.Presets{ SourceLinkedDeployment: tt.initialValue, }, - Workspace: config.Workspace{ - FilePath: remotePath, - }, }, } @@ -458,7 +446,6 @@ func TestApplyPresetsSourceLinkedDeployment(t *testing.T) { require.Equal(t, tt.expectedWarning, diags[0].Summary) } - require.Equal(t, tt.expectedFilePath, b.Config.Workspace.FilePath) require.Equal(t, tt.expectedValue, b.Config.Presets.SourceLinkedDeployment) }) } diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index d6985bda..bf2129f9 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -524,14 +524,12 @@ func TestSourceLinkedDeploymentEnabled(t *testing.T) { b, diags := processSourceLinkedBundle(t, true) require.NoError(t, diags.Error()) assert.True(t, *b.Config.Presets.SourceLinkedDeployment) - assert.Equal(t, b.Config.Workspace.FilePath, b.SyncRootPath) } func TestSourceLinkedDeploymentDisabled(t *testing.T) { b, diags := processSourceLinkedBundle(t, false) require.NoError(t, diags.Error()) assert.False(t, *b.Config.Presets.SourceLinkedDeployment) - assert.NotEqual(t, b.Config.Workspace.FilePath, b.SyncRootPath) } func processSourceLinkedBundle(t *testing.T, presetEnabled bool) (*bundle.Bundle, diag.Diagnostics) { @@ -542,7 +540,6 @@ func processSourceLinkedBundle(t *testing.T, presetEnabled bool) (*bundle.Bundle b := mockBundle(config.Development) workspacePath := "/Workspace/lennart@company.com/" - b.SyncRoot = vfs.MustNew(workspacePath) b.SyncRootPath = workspacePath b.Config.Presets.SourceLinkedDeployment = &presetEnabled diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index 321fa5b3..1e2484c7 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/notebook" @@ -103,8 +104,13 @@ func (t *translateContext) rewritePath( return fmt.Errorf("path %s is not contained in sync root path", localPath) } - // Prefix remote path with its remote root path. - remotePath := path.Join(t.b.Config.Workspace.FilePath, filepath.ToSlash(localRelPath)) + var workspacePath string + if config.IsExplicitlyEnabled(t.b.Config.Presets.SourceLinkedDeployment) { + workspacePath = t.b.SyncRootPath + } else { + workspacePath = t.b.Config.Workspace.FilePath + } + remotePath := path.Join(workspacePath, filepath.ToSlash(localRelPath)) // Convert local path into workspace path via specified function. interp, err := fn(*p, localPath, localRelPath, remotePath) From 8c8fb35861a7fb3abe3d6930c20e893f6e6f0213 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Tue, 19 Nov 2024 10:46:40 +0100 Subject: [PATCH 23/30] fix: Add explicit warning when using python wheel wrappers with source-linked deployment --- bundle/trampoline/python_dbr_warning.go | 4 ++++ bundle/trampoline/python_dbr_warning_test.go | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/bundle/trampoline/python_dbr_warning.go b/bundle/trampoline/python_dbr_warning.go index f62e9eab..cf3e9aeb 100644 --- a/bundle/trampoline/python_dbr_warning.go +++ b/bundle/trampoline/python_dbr_warning.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/libraries" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" @@ -22,6 +23,9 @@ func WrapperWarning() bundle.Mutator { func (m *wrapperWarning) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { if isPythonWheelWrapperOn(b) { + if config.IsExplicitlyEnabled(b.Config.Presets.SourceLinkedDeployment) { + return diag.Warningf("Python wheel notebook wrapper is not available when using source-linked deployment mode. You can disable this mode by setting 'presets.source_linked_deployment: false'") + } return nil } diff --git a/bundle/trampoline/python_dbr_warning_test.go b/bundle/trampoline/python_dbr_warning_test.go index d293c947..aef1b37b 100644 --- a/bundle/trampoline/python_dbr_warning_test.go +++ b/bundle/trampoline/python_dbr_warning_test.go @@ -335,6 +335,24 @@ func TestNoWarningWhenPythonWheelWrapperIsOn(t *testing.T) { require.NoError(t, diags.Error()) } +func TestPythonWheelWithSourceLinkedDeployment(t *testing.T) { + enabled := true + b := &bundle.Bundle{ + Config: config.Root{ + Experimental: &config.Experimental{ + PythonWheelWrapper: true, + }, + Presets: config.Presets{ + SourceLinkedDeployment: &enabled, + }, + }, + } + + diags := bundle.Apply(context.Background(), b, WrapperWarning()) + require.NoError(t, diags.Error()) + require.Contains(t, diags[0].Summary, "Python wheel notebook wrapper is not available when using source-linked deployment mode") +} + func TestSparkVersionLowerThanExpected(t *testing.T) { testCases := map[string]bool{ "13.1.x-scala2.12": false, From 234d97179009c4cf1dadc8ade26ea321c6b85531 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Tue, 19 Nov 2024 11:47:57 +0100 Subject: [PATCH 24/30] fix: Cleanup in test --- bundle/config/mutator/apply_presets_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/bundle/config/mutator/apply_presets_test.go b/bundle/config/mutator/apply_presets_test.go index dc56c27f..f11a45d6 100644 --- a/bundle/config/mutator/apply_presets_test.go +++ b/bundle/config/mutator/apply_presets_test.go @@ -10,7 +10,6 @@ import ( "github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/dbr" - "github.com/databricks/cli/libs/vfs" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/stretchr/testify/require" @@ -48,7 +47,6 @@ func TestApplyPresetsPrefix(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { b := &bundle.Bundle{ - SyncRoot: vfs.MustNew(t.TempDir()), Config: config.Root{ Resources: config.Resources{ Jobs: map[string]*resources.Job{ From 5d09070fcb6d3a9eaaa39ec489bcc79ca34b1169 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Tue, 19 Nov 2024 11:50:28 +0100 Subject: [PATCH 25/30] fix: Cleanup --- bundle/config/mutator/process_target_mode.go | 3 +-- bundle/config/validate/folder_permissions_test.go | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index 77bb54cb..df0136fa 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -59,8 +59,7 @@ func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) { } if !config.IsExplicitlyDisabled(t.SourceLinkedDeployment) { - root := b.SyncRootPath - isInWorkspace := strings.HasPrefix(root, "/Workspace/") + isInWorkspace := strings.HasPrefix(b.SyncRootPath, "/Workspace/") if isInWorkspace && dbr.RunsOnRuntime(ctx) { enabled := true t.SourceLinkedDeployment = &enabled diff --git a/bundle/config/validate/folder_permissions_test.go b/bundle/config/validate/folder_permissions_test.go index f609724e..8e68c9fb 100644 --- a/bundle/config/validate/folder_permissions_test.go +++ b/bundle/config/validate/folder_permissions_test.go @@ -18,7 +18,6 @@ import ( func TestFolderPermissionsInheritedWhenRootPathDoesNotExist(t *testing.T) { b := &bundle.Bundle{ - SyncRootPath: t.TempDir(), Config: config.Root{ Workspace: config.Workspace{ RootPath: "/Workspace/Users/foo@bar.com", From 8ee8de554b1950d31ecb2f30bc2a4d34cbfdaff1 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Tue, 19 Nov 2024 12:46:31 +0100 Subject: [PATCH 26/30] test: Added path translation test cases for source-linked --- bundle/config/mutator/translate_paths_test.go | 156 ++++++++++++++++++ 1 file changed, 156 insertions(+) diff --git a/bundle/config/mutator/translate_paths_test.go b/bundle/config/mutator/translate_paths_test.go index 9d655b27..02632967 100644 --- a/bundle/config/mutator/translate_paths_test.go +++ b/bundle/config/mutator/translate_paths_test.go @@ -787,3 +787,159 @@ func TestTranslatePathWithComplexVariables(t *testing.T) { b.Config.Resources.Jobs["job"].Tasks[0].Libraries[0].Whl, ) } + +func TestTranslatePathsWithSourceLinkedDeployment(t *testing.T) { + dir := t.TempDir() + touchNotebookFile(t, filepath.Join(dir, "my_job_notebook.py")) + touchNotebookFile(t, filepath.Join(dir, "my_pipeline_notebook.py")) + touchEmptyFile(t, filepath.Join(dir, "my_python_file.py")) + touchEmptyFile(t, filepath.Join(dir, "dist", "task.jar")) + touchEmptyFile(t, filepath.Join(dir, "requirements.txt")) + + enabled := true + b := &bundle.Bundle{ + SyncRootPath: dir, + SyncRoot: vfs.MustNew(dir), + Config: config.Root{ + Workspace: config.Workspace{ + FilePath: "/bundle", + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job": { + JobSettings: &jobs.JobSettings{ + Tasks: []jobs.Task{ + { + NotebookTask: &jobs.NotebookTask{ + NotebookPath: "./my_job_notebook.py", + }, + Libraries: []compute.Library{ + {Whl: "./dist/task.whl"}, + }, + }, + { + NotebookTask: &jobs.NotebookTask{ + NotebookPath: "/Users/jane.doe@databricks.com/absolute_remote.py", + }, + }, + { + NotebookTask: &jobs.NotebookTask{ + NotebookPath: "./my_job_notebook.py", + }, + Libraries: []compute.Library{ + {Requirements: "./requirements.txt"}, + }, + }, + { + SparkPythonTask: &jobs.SparkPythonTask{ + PythonFile: "./my_python_file.py", + }, + }, + { + SparkJarTask: &jobs.SparkJarTask{ + MainClassName: "HelloWorld", + }, + Libraries: []compute.Library{ + {Jar: "./dist/task.jar"}, + }, + }, + { + SparkJarTask: &jobs.SparkJarTask{ + MainClassName: "HelloWorldRemote", + }, + Libraries: []compute.Library{ + {Jar: "dbfs:/bundle/dist/task_remote.jar"}, + }, + }, + }, + }, + }, + }, + Pipelines: map[string]*resources.Pipeline{ + "pipeline": { + PipelineSpec: &pipelines.PipelineSpec{ + Libraries: []pipelines.PipelineLibrary{ + { + Notebook: &pipelines.NotebookLibrary{ + Path: "./my_pipeline_notebook.py", + }, + }, + { + Notebook: &pipelines.NotebookLibrary{ + Path: "/Users/jane.doe@databricks.com/absolute_remote.py", + }, + }, + { + File: &pipelines.FileLibrary{ + Path: "./my_python_file.py", + }, + }, + }, + }, + }, + }, + }, + Presets: config.Presets{ + SourceLinkedDeployment: &enabled, + }, + }, + } + + bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(dir, "resource.yml")}}) + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + require.NoError(t, diags.Error()) + + // updated to source path + assert.Equal( + t, + filepath.Join(dir, "my_job_notebook"), + b.Config.Resources.Jobs["job"].Tasks[0].NotebookTask.NotebookPath, + ) + assert.Equal( + t, + filepath.Join(dir, "requirements.txt"), + b.Config.Resources.Jobs["job"].Tasks[2].Libraries[0].Requirements, + ) + assert.Equal( + t, + filepath.Join(dir, "my_python_file.py"), + b.Config.Resources.Jobs["job"].Tasks[3].SparkPythonTask.PythonFile, + ) + assert.Equal( + t, + filepath.Join(dir, "my_pipeline_notebook"), + b.Config.Resources.Pipelines["pipeline"].Libraries[0].Notebook.Path, + ) + assert.Equal( + t, + filepath.Join(dir, "my_python_file.py"), + b.Config.Resources.Pipelines["pipeline"].Libraries[2].File.Path, + ) + + // left as is + assert.Equal( + t, + "dist/task.whl", + b.Config.Resources.Jobs["job"].Tasks[0].Libraries[0].Whl, + ) + assert.Equal( + t, + "/Users/jane.doe@databricks.com/absolute_remote.py", + b.Config.Resources.Jobs["job"].Tasks[1].NotebookTask.NotebookPath, + ) + assert.Equal( + t, + filepath.Join("dist", "task.jar"), + b.Config.Resources.Jobs["job"].Tasks[4].Libraries[0].Jar, + ) + assert.Equal( + t, + "dbfs:/bundle/dist/task_remote.jar", + b.Config.Resources.Jobs["job"].Tasks[5].Libraries[0].Jar, + ) + assert.Equal( + t, + "/Users/jane.doe@databricks.com/absolute_remote.py", + b.Config.Resources.Pipelines["pipeline"].Libraries[1].Notebook.Path, + ) +} From 5d04569112c3b886bd919f355285787489f3b799 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Tue, 19 Nov 2024 13:15:52 +0100 Subject: [PATCH 27/30] fix: Windows tests --- bundle/config/mutator/translate_paths_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/bundle/config/mutator/translate_paths_test.go b/bundle/config/mutator/translate_paths_test.go index 02632967..45a8b62f 100644 --- a/bundle/config/mutator/translate_paths_test.go +++ b/bundle/config/mutator/translate_paths_test.go @@ -811,7 +811,7 @@ func TestTranslatePathsWithSourceLinkedDeployment(t *testing.T) { Tasks: []jobs.Task{ { NotebookTask: &jobs.NotebookTask{ - NotebookPath: "./my_job_notebook.py", + NotebookPath: "my_job_notebook.py", }, Libraries: []compute.Library{ {Whl: "./dist/task.whl"}, @@ -824,15 +824,15 @@ func TestTranslatePathsWithSourceLinkedDeployment(t *testing.T) { }, { NotebookTask: &jobs.NotebookTask{ - NotebookPath: "./my_job_notebook.py", + NotebookPath: "my_job_notebook.py", }, Libraries: []compute.Library{ - {Requirements: "./requirements.txt"}, + {Requirements: "requirements.txt"}, }, }, { SparkPythonTask: &jobs.SparkPythonTask{ - PythonFile: "./my_python_file.py", + PythonFile: "my_python_file.py", }, }, { @@ -861,7 +861,7 @@ func TestTranslatePathsWithSourceLinkedDeployment(t *testing.T) { Libraries: []pipelines.PipelineLibrary{ { Notebook: &pipelines.NotebookLibrary{ - Path: "./my_pipeline_notebook.py", + Path: "my_pipeline_notebook.py", }, }, { @@ -871,7 +871,7 @@ func TestTranslatePathsWithSourceLinkedDeployment(t *testing.T) { }, { File: &pipelines.FileLibrary{ - Path: "./my_python_file.py", + Path: "my_python_file.py", }, }, }, @@ -919,7 +919,7 @@ func TestTranslatePathsWithSourceLinkedDeployment(t *testing.T) { // left as is assert.Equal( t, - "dist/task.whl", + filepath.Join("dist", "task.whl"), b.Config.Resources.Jobs["job"].Tasks[0].Libraries[0].Whl, ) assert.Equal( From 530a2a9bab51ed367ca2ec56210b4a680d85a0cd Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Tue, 19 Nov 2024 13:33:02 +0100 Subject: [PATCH 28/30] fix: Skipping path translation tests for windows --- bundle/config/mutator/translate_paths_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/bundle/config/mutator/translate_paths_test.go b/bundle/config/mutator/translate_paths_test.go index 45a8b62f..a2032f81 100644 --- a/bundle/config/mutator/translate_paths_test.go +++ b/bundle/config/mutator/translate_paths_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "runtime" "strings" "testing" @@ -789,6 +790,10 @@ func TestTranslatePathWithComplexVariables(t *testing.T) { } func TestTranslatePathsWithSourceLinkedDeployment(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("this test is not applicable on Windows because source-linked mode works only in the Databricks Workspace") + } + dir := t.TempDir() touchNotebookFile(t, filepath.Join(dir, "my_job_notebook.py")) touchNotebookFile(t, filepath.Join(dir, "my_pipeline_notebook.py")) From aba35ae3fe637e3557e6d2ac3af467ceed1c0c50 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Tue, 19 Nov 2024 22:08:45 +0100 Subject: [PATCH 29/30] fix: Replaced diag warning message with logger and removed unnecessary test --- bundle/trampoline/python_dbr_warning.go | 3 ++- bundle/trampoline/python_dbr_warning_test.go | 18 ------------------ 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/bundle/trampoline/python_dbr_warning.go b/bundle/trampoline/python_dbr_warning.go index cf3e9aeb..e93bc321 100644 --- a/bundle/trampoline/python_dbr_warning.go +++ b/bundle/trampoline/python_dbr_warning.go @@ -8,6 +8,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/libraries" + "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -24,7 +25,7 @@ func WrapperWarning() bundle.Mutator { func (m *wrapperWarning) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { if isPythonWheelWrapperOn(b) { if config.IsExplicitlyEnabled(b.Config.Presets.SourceLinkedDeployment) { - return diag.Warningf("Python wheel notebook wrapper is not available when using source-linked deployment mode. You can disable this mode by setting 'presets.source_linked_deployment: false'") + cmdio.LogString(ctx, "Python wheel notebook wrapper is not available when using source-linked deployment mode. You can disable this mode by setting 'presets.source_linked_deployment: false'") } return nil } diff --git a/bundle/trampoline/python_dbr_warning_test.go b/bundle/trampoline/python_dbr_warning_test.go index aef1b37b..d293c947 100644 --- a/bundle/trampoline/python_dbr_warning_test.go +++ b/bundle/trampoline/python_dbr_warning_test.go @@ -335,24 +335,6 @@ func TestNoWarningWhenPythonWheelWrapperIsOn(t *testing.T) { require.NoError(t, diags.Error()) } -func TestPythonWheelWithSourceLinkedDeployment(t *testing.T) { - enabled := true - b := &bundle.Bundle{ - Config: config.Root{ - Experimental: &config.Experimental{ - PythonWheelWrapper: true, - }, - Presets: config.Presets{ - SourceLinkedDeployment: &enabled, - }, - }, - } - - diags := bundle.Apply(context.Background(), b, WrapperWarning()) - require.NoError(t, diags.Error()) - require.Contains(t, diags[0].Summary, "Python wheel notebook wrapper is not available when using source-linked deployment mode") -} - func TestSparkVersionLowerThanExpected(t *testing.T) { testCases := map[string]bool{ "13.1.x-scala2.12": false, From eede522d879ae6ccfd88d73695b1410f64c61bd0 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Wed, 20 Nov 2024 11:34:35 +0100 Subject: [PATCH 30/30] Update bundle/config/presets.go Co-authored-by: Pieter Noordhuis --- bundle/config/presets.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/presets.go b/bundle/config/presets.go index 14c0d8a2..30f56c0f 100644 --- a/bundle/config/presets.go +++ b/bundle/config/presets.go @@ -19,7 +19,7 @@ type Presets struct { // SourceLinkedDeployment indicates whether source-linked deployment is enabled. Works only in Databricks Workspace // When set to true, resources created during deployment will point to source files in the workspace instead of their workspace copies. - // No resources will be uploaded to workspace + // File synchronization to ${workspace.file_path} is skipped. SourceLinkedDeployment *bool `json:"source_linked_deployment,omitempty"` // Tags to add to all resources.