From 42cad1413a99a269fd6dba91b7c773de8ba3a680 Mon Sep 17 00:00:00 2001 From: Ilya Kuznetsov Date: Mon, 6 Jan 2025 15:07:13 +0100 Subject: [PATCH] feat: Change metadata.file_path, print warning when file_path is explicitly set --- .../apply_source_linked_deployment_preset.go | 17 +++++- ...ly_source_linked_deployment_preset_test.go | 52 +++++++++++-------- bundle/deploy/metadata/compute.go | 3 ++ bundle/deploy/metadata/compute_test.go | 21 ++++++++ bundle/phases/initialize.go | 8 +-- 5 files changed, 73 insertions(+), 28 deletions(-) diff --git a/bundle/config/mutator/apply_source_linked_deployment_preset.go b/bundle/config/mutator/apply_source_linked_deployment_preset.go index 5724e2f5f..78ccc5322 100644 --- a/bundle/config/mutator/apply_source_linked_deployment_preset.go +++ b/bundle/config/mutator/apply_source_linked_deployment_preset.go @@ -29,10 +29,10 @@ func (m *applySourceLinkedDeploymentPreset) Apply(ctx context.Context, b *bundle var diags diag.Diagnostics isDatabricksWorkspace := dbr.RunsOnRuntime(ctx) && strings.HasPrefix(b.SyncRootPath, "/Workspace/") + target := b.Config.Bundle.Target if config.IsExplicitlyEnabled((b.Config.Presets.SourceLinkedDeployment)) { if !isDatabricksWorkspace { - target := b.Config.Bundle.Target path := dyn.NewPath(dyn.Key("targets"), dyn.Key(target), dyn.Key("presets"), dyn.Key("source_linked_deployment")) diags = diags.Append( diag.Diagnostic{ @@ -56,5 +56,20 @@ func (m *applySourceLinkedDeploymentPreset) Apply(ctx context.Context, b *bundle b.Config.Presets.SourceLinkedDeployment = &enabled } + if b.Config.Workspace.FilePath != "" && config.IsExplicitlyEnabled(b.Config.Presets.SourceLinkedDeployment) { + path := dyn.NewPath(dyn.Key("targets"), dyn.Key(target), dyn.Key("workspace"), dyn.Key("file_path")) + + diags = diags.Append( + diag.Diagnostic{ + Severity: diag.Warning, + Summary: "workspace.file_path setting will be ignored in source-linked deployment mode", + Paths: []dyn.Path{ + path[2:], + }, + Locations: b.Config.GetLocations(path[2:].String()), + }, + ) + } + return diags } diff --git a/bundle/config/mutator/apply_source_linked_deployment_preset_test.go b/bundle/config/mutator/apply_source_linked_deployment_preset_test.go index 0ca83e0e3..1b74fd8e9 100644 --- a/bundle/config/mutator/apply_source_linked_deployment_preset_test.go +++ b/bundle/config/mutator/apply_source_linked_deployment_preset_test.go @@ -25,81 +25,87 @@ func TestApplyPresetsSourceLinkedDeployment(t *testing.T) { workspacePath := "/Workspace/user.name@company.com" tests := []struct { - bundlePath string - ctx context.Context name string - mode config.Mode + ctx context.Context + mutateBundle func(b *bundle.Bundle) initialValue *bool expectedValue *bool expectedWarning string }{ { name: "preset enabled, bundle in Workspace, databricks runtime", - bundlePath: workspacePath, ctx: dbr.MockRuntime(testContext, true), - mode: config.Production, initialValue: &enabled, expectedValue: &enabled, }, { - name: "preset enabled, bundle not in Workspace, databricks runtime", - bundlePath: "/Users/user.name@company.com", - ctx: dbr.MockRuntime(testContext, true), - mode: config.Production, + name: "preset enabled, bundle not in Workspace, databricks runtime", + ctx: dbr.MockRuntime(testContext, true), + mutateBundle: func(b *bundle.Bundle) { + b.SyncRootPath = "/Users/user.name@company.com" + }, 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), - mode: config.Production, 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), - mode: config.Production, initialValue: &disabled, expectedValue: &disabled, }, { name: "preset nil, bundle in Workspace, databricks runtime", - bundlePath: workspacePath, ctx: dbr.MockRuntime(testContext, true), - mode: config.Production, initialValue: nil, expectedValue: nil, }, { - name: "preset nil, dev mode true, bundle in Workspace, databricks runtime", - bundlePath: workspacePath, - ctx: dbr.MockRuntime(testContext, true), - mode: config.Development, + name: "preset nil, dev mode true, bundle in Workspace, databricks runtime", + ctx: dbr.MockRuntime(testContext, true), + mutateBundle: func(b *bundle.Bundle) { + b.Config.Bundle.Mode = config.Development + }, initialValue: nil, expectedValue: &enabled, }, + { + name: "preset enabled, workspace.file_path is defined by user", + ctx: dbr.MockRuntime(testContext, true), + mutateBundle: func(b *bundle.Bundle) { + b.Config.Workspace.FilePath = "file_path" + }, + initialValue: &enabled, + expectedValue: &enabled, + expectedWarning: "workspace.file_path setting will be ignored in source-linked deployment mode", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { b := &bundle.Bundle{ - SyncRootPath: tt.bundlePath, + SyncRootPath: workspacePath, Config: config.Root{ Presets: config.Presets{ SourceLinkedDeployment: tt.initialValue, }, - Bundle: config.Bundle{ - Mode: tt.mode, - }, }, } + if tt.mutateBundle != nil { + tt.mutateBundle(b) + } + bundletest.SetLocation(b, "presets.source_linked_deployment", []dyn.Location{{File: "databricks.yml"}}) + bundletest.SetLocation(b, "workspace.file_path", []dyn.Location{{File: "databricks.yml"}}) + diags := bundle.Apply(tt.ctx, b, mutator.ApplySourceLinkedDeploymentPreset()) if diags.HasError() { t.Fatalf("unexpected error: %v", diags) diff --git a/bundle/deploy/metadata/compute.go b/bundle/deploy/metadata/compute.go index bc8767de4..b47baa6b2 100644 --- a/bundle/deploy/metadata/compute.go +++ b/bundle/deploy/metadata/compute.go @@ -54,5 +54,8 @@ func (m *compute) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { // Set file upload destination of the bundle in metadata b.Metadata.Config.Workspace.FilePath = b.Config.Workspace.FilePath + if config.IsExplicitlyEnabled(b.Config.Presets.SourceLinkedDeployment) { + b.Metadata.Config.Workspace.FilePath = b.SyncRootPath + } return nil } diff --git a/bundle/deploy/metadata/compute_test.go b/bundle/deploy/metadata/compute_test.go index 2c2c72376..c6fa9bddb 100644 --- a/bundle/deploy/metadata/compute_test.go +++ b/bundle/deploy/metadata/compute_test.go @@ -97,3 +97,24 @@ func TestComputeMetadataMutator(t *testing.T) { assert.Equal(t, expectedMetadata, b.Metadata) } + +func TestComputeMetadataMutatorSourceLinked(t *testing.T) { + syncRootPath := "/Users/shreyas.goenka@databricks.com/source" + enabled := true + b := &bundle.Bundle{ + SyncRootPath: syncRootPath, + Config: config.Root{ + Presets: config.Presets{ + SourceLinkedDeployment: &enabled, + }, + Workspace: config.Workspace{ + FilePath: "/Users/shreyas.goenka@databricks.com/files", + }, + }, + } + + diags := bundle.Apply(context.Background(), b, Compute()) + require.NoError(t, diags.Error()) + + assert.Equal(t, syncRootPath, b.Metadata.Config.Workspace.FilePath) +} diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index ce90b3261..1a8217ecb 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -41,6 +41,10 @@ func Initialize() bundle.Mutator { mutator.PopulateCurrentUser(), mutator.LoadGitDetails(), + // This mutator needs to be run before variable interpolation and defining default workspace paths + // because it affects how workspace variables are resolved. + mutator.ApplySourceLinkedDeploymentPreset(), + mutator.DefineDefaultWorkspaceRoot(), mutator.ExpandWorkspaceRoot(), mutator.DefineDefaultWorkspacePaths(), @@ -52,10 +56,6 @@ func Initialize() bundle.Mutator { mutator.SetVariables(), - // This mutator needs to be run before variable interpolation because it affects - // how workspace variables are resolved. - mutator.ApplySourceLinkedDeploymentPreset(), - // Intentionally placed before ResolveVariableReferencesInLookup, ResolveResourceReferences, // ResolveVariableReferencesInComplexVariables and ResolveVariableReferences. // See what is expected in PythonMutatorPhaseInit doc