From b7e43af4e2b98d57a4d1d591ad0458743825c704 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 20 Mar 2024 16:32:21 +0100 Subject: [PATCH 1/4] Disable locking for development mode --- bundle/deploy/lock/acquire.go | 5 +++++ bundle/deploy/lock/release.go | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/bundle/deploy/lock/acquire.go b/bundle/deploy/lock/acquire.go index 69e6663fc7..4db6397b41 100644 --- a/bundle/deploy/lock/acquire.go +++ b/bundle/deploy/lock/acquire.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/locker" "github.com/databricks/cli/libs/log" @@ -39,6 +40,10 @@ func (m *acquire) Apply(ctx context.Context, b *bundle.Bundle) error { log.Infof(ctx, "Skipping; locking is disabled") return nil } + if b.Config.Bundle.Mode == config.Development { + log.Infof(ctx, "Skipping; locking is disabled in development mode") + return nil + } err := m.init(b) if err != nil { diff --git a/bundle/deploy/lock/release.go b/bundle/deploy/lock/release.go index 4ea47c2f97..b05f5b7cf5 100644 --- a/bundle/deploy/lock/release.go +++ b/bundle/deploy/lock/release.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/locker" "github.com/databricks/cli/libs/log" ) @@ -36,6 +37,10 @@ func (m *release) Apply(ctx context.Context, b *bundle.Bundle) error { log.Infof(ctx, "Skipping; locking is disabled") return nil } + if b.Config.Bundle.Mode == config.Development { + log.Infof(ctx, "Skipping; locking is disabled in development mode") + return nil + } // Return early if the locker is not set. // It is likely an error occurred prior to initialization of the locker instance. From 5221b31f8359d306bd8018cd0e8e6920a1c55923 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Fri, 29 Mar 2024 14:27:54 +0100 Subject: [PATCH 2/4] Disable log from process_target_mode to allow configurability --- bundle/config/deployment.go | 2 +- bundle/config/lock.go | 13 +++++++++++- bundle/config/mutator/process_target_mode.go | 19 +++++++++++++++++- .../mutator/process_target_mode_test.go | 20 +++++++++++++++++++ bundle/config/root.go | 12 +++++++++++ bundle/deploy/lock/acquire.go | 5 ----- bundle/deploy/lock/release.go | 5 ----- 7 files changed, 63 insertions(+), 13 deletions(-) diff --git a/bundle/config/deployment.go b/bundle/config/deployment.go index f89c7b3eee..7f0f57a8c3 100644 --- a/bundle/config/deployment.go +++ b/bundle/config/deployment.go @@ -6,5 +6,5 @@ type Deployment struct { FailOnActiveRuns bool `json:"fail_on_active_runs,omitempty"` // Lock configures locking behavior on deployment. - Lock Lock `json:"lock" bundle:"readonly"` + Lock Lock `json:"lock"` } diff --git a/bundle/config/lock.go b/bundle/config/lock.go index 760099a95c..256d668f8e 100644 --- a/bundle/config/lock.go +++ b/bundle/config/lock.go @@ -1,7 +1,7 @@ package config type Lock struct { - // Enabled toggles deployment lock. True by default. + // Enabled toggles deployment lock. True by default except in development mode. // Use a pointer value so that only explicitly configured values are set // and we don't merge configuration with zero-initialized values. Enabled *bool `json:"enabled,omitempty"` @@ -11,9 +11,20 @@ type Lock struct { Force bool `json:"force,omitempty"` } +// IsEnabled checks if the deployment lock is enabled. func (lock Lock) IsEnabled() bool { if lock.Enabled != nil { return *lock.Enabled } return true } + +// IsAlwaysEnabled checks if the deployment lock is explicitly enabled. +// Only returns true if locking is explicitly set using a command-line +// flag or configuration file. +func (lock Lock) IsExplicitlyEnabled() bool { + if lock.Enabled != nil { + return *lock.Enabled + } + return false +} diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index e57509452e..6d4cf692e9 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -9,6 +9,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/auth" + "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/ml" @@ -29,12 +30,20 @@ func (m *processTargetMode) Name() string { // Mark all resources as being for 'development' purposes, i.e. // changing their their name, adding tags, and (in the future) // marking them as 'hidden' in the UI. -func transformDevelopmentMode(b *bundle.Bundle) error { +func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) error { r := b.Config.Resources shortName := b.Config.Workspace.CurrentUser.ShortName prefix := "[dev " + shortName + "] " + if !b.Config.Bundle.Deployment.Lock.IsExplicitlyEnabled() { + log.Infof(ctx, "Development mode: disabling deployment lock since bundle.deployment.lock.enabled is not set to true") + err := disableDeploymentLock(b) + if err != nil { + return err + } + } + // Generate a normalized version of the short name that can be used as a tag value. tagValue := b.Tagging.NormalizeValue(shortName) @@ -100,6 +109,14 @@ func transformDevelopmentMode(b *bundle.Bundle) error { return nil } +func disableDeploymentLock(b *bundle.Bundle) error { + return b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.Map(v, "bundle.deployment.lock", func(_ dyn.Path, v dyn.Value) (dyn.Value, error) { + return dyn.Set(v, "enabled", dyn.V(false)) + }) + }) +} + func validateDevelopmentMode(b *bundle.Bundle) error { if path := findNonUserPath(b); path != "" { return fmt.Errorf("%s must start with '~/' or contain the current username when using 'mode: development'", path) diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index a5f61284c7..aa2a1789bd 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -301,3 +301,23 @@ func TestAllResourcesRenamed(t *testing.T) { } } } + +func TestDisableLocking(t *testing.T) { + ctx := context.Background() + b := mockBundle(config.Development) + + err := transformDevelopmentMode(ctx, b) + require.NoError(t, err) + assert.False(t, b.Config.Bundle.Deployment.Lock.IsEnabled()) +} + +func TestDisableLockingDisabled(t *testing.T) { + ctx := context.Background() + b := mockBundle(config.Development) + explicitlyEnabled := true + b.Config.Bundle.Deployment.Lock.Enabled = &explicitlyEnabled + + err := transformDevelopmentMode(ctx, b) + require.NoError(t, err) + assert.True(t, b.Config.Bundle.Deployment.Lock.IsEnabled(), "Deployment lock should remain enabled in development mode when explicitly enabled") +} diff --git a/bundle/config/root.go b/bundle/config/root.go index 8e1ff65077..fb1c3ade1e 100644 --- a/bundle/config/root.go +++ b/bundle/config/root.go @@ -157,6 +157,18 @@ func (r *Root) updateWithDynamicValue(nv dyn.Value) error { return nil } +// Mutate applies a transformation to the dynamic configuration value of a Root object. +// +// Parameters: +// - fn: A function that mutates a dyn.Value object +// +// Example usage, setting bundle.deployment.lock.enabled to false: +// +// err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { +// return dyn.Map(v, "bundle.deployment.lock", func(_ dyn.Path, v dyn.Value) (dyn.Value, error) { +// return dyn.Set(v, "enabled", dyn.V(false)) +// }) +// }) func (r *Root) Mutate(fn func(dyn.Value) (dyn.Value, error)) error { err := r.initializeDynamicValue() if err != nil { diff --git a/bundle/deploy/lock/acquire.go b/bundle/deploy/lock/acquire.go index 4db6397b41..69e6663fc7 100644 --- a/bundle/deploy/lock/acquire.go +++ b/bundle/deploy/lock/acquire.go @@ -6,7 +6,6 @@ import ( "fmt" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/locker" "github.com/databricks/cli/libs/log" @@ -40,10 +39,6 @@ func (m *acquire) Apply(ctx context.Context, b *bundle.Bundle) error { log.Infof(ctx, "Skipping; locking is disabled") return nil } - if b.Config.Bundle.Mode == config.Development { - log.Infof(ctx, "Skipping; locking is disabled in development mode") - return nil - } err := m.init(b) if err != nil { diff --git a/bundle/deploy/lock/release.go b/bundle/deploy/lock/release.go index b05f5b7cf5..4ea47c2f97 100644 --- a/bundle/deploy/lock/release.go +++ b/bundle/deploy/lock/release.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/locker" "github.com/databricks/cli/libs/log" ) @@ -37,10 +36,6 @@ func (m *release) Apply(ctx context.Context, b *bundle.Bundle) error { log.Infof(ctx, "Skipping; locking is disabled") return nil } - if b.Config.Bundle.Mode == config.Development { - log.Infof(ctx, "Skipping; locking is disabled in development mode") - return nil - } // Return early if the locker is not set. // It is likely an error occurred prior to initialization of the locker instance. From b38a4e90644e770563d7d2d5324868ea2226c7d7 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 3 Apr 2024 20:49:11 +0200 Subject: [PATCH 3/4] Fix tests --- bundle/config/mutator/process_target_mode.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index dfa2dadc0a..8e70fab73b 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -31,11 +31,6 @@ func (m *processTargetMode) Name() string { // changing their their name, adding tags, and (in the future) // marking them as 'hidden' in the UI. func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - r := b.Config.Resources - - shortName := b.Config.Workspace.CurrentUser.ShortName - prefix := "[dev " + shortName + "] " - if !b.Config.Bundle.Deployment.Lock.IsExplicitlyEnabled() { log.Infof(ctx, "Development mode: disabling deployment lock since bundle.deployment.lock.enabled is not set to true") err := disableDeploymentLock(b) @@ -44,6 +39,10 @@ func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) diag.Diagno } } + r := b.Config.Resources + shortName := b.Config.Workspace.CurrentUser.ShortName + prefix := "[dev " + shortName + "] " + // Generate a normalized version of the short name that can be used as a tag value. tagValue := b.Tagging.NormalizeValue(shortName) From a68c95c011156b0c7bfd48bf218e85a39fedb28b Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 16 Apr 2024 14:30:17 +0200 Subject: [PATCH 4/4] Update bundle/config/lock.go Co-authored-by: Andrew Nester --- bundle/config/lock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/config/lock.go b/bundle/config/lock.go index 256d668f8e..10e9e1c9c5 100644 --- a/bundle/config/lock.go +++ b/bundle/config/lock.go @@ -19,7 +19,7 @@ func (lock Lock) IsEnabled() bool { return true } -// IsAlwaysEnabled checks if the deployment lock is explicitly enabled. +// IsExplicitlyEnabled checks if the deployment lock is explicitly enabled. // Only returns true if locking is explicitly set using a command-line // flag or configuration file. func (lock Lock) IsExplicitlyEnabled() bool {