Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion bundle/config/experimental.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ type Experimental struct {
//
// Only for bundles with no deployed resources yet: DMS becomes the source of
// truth for their state.
DeploymentHistory bool `json:"deployment_history,omitempty"`
// An explicit value takes priority over the environment setting.
DeploymentHistory *bool `json:"deployment_history,omitempty"`
}

type Python struct {
Expand Down
9 changes: 5 additions & 4 deletions bundle/env/dms.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import "context"
const DeploymentHistoryVariable = "DATABRICKS_BUNDLE_DEPLOYMENT_HISTORY"

// RecordsDeploymentHistory reports whether recording is on, from config or env var. Single predicate
// for all recording code paths, keeping them in sync. The env var only counts as "true": anything
// for all recording code paths, keeping them in sync. Explicit config takes priority over the env var.
// The env var only counts as "true": anything
// else - a typo like "TRUE" or "yes" - leaves recording off rather than silently enabling it.
func RecordsDeploymentHistory(ctx context.Context, configured bool) bool {
if configured {
return true
func RecordsDeploymentHistory(ctx context.Context, configured *bool) bool {
if configured != nil {
return *configured
}
value, _ := get(ctx, []string{DeploymentHistoryVariable})
return value == "true"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: typically for env var any non-empty string is true. e.g. DATABRICKS_BUNDLE_DEPLOYMENT_HISTORY=1

Expand Down
32 changes: 23 additions & 9 deletions bundle/env/dms_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,34 @@ func TestDeploymentHistoryEnv(t *testing.T) {
{"yes", false},
} {
ctx := env.Set(t.Context(), DeploymentHistoryVariable, tc.value)
assert.Equal(t, tc.want, RecordsDeploymentHistory(ctx, false), "value %q", tc.value)
assert.Equal(t, tc.want, RecordsDeploymentHistory(ctx, nil), "value %q", tc.value)
}
}

func TestDeploymentHistoryEnvUnset(t *testing.T) {
assert.False(t, RecordsDeploymentHistory(t.Context(), false))
assert.False(t, RecordsDeploymentHistory(t.Context(), nil))
}

func TestRecordsDeploymentHistory(t *testing.T) {
// The bundle setting alone is enough, and so is the environment; the env var
// exists so the acceptance suite can record without touching every databricks.yml.
assert.True(t, RecordsDeploymentHistory(t.Context(), true))
assert.False(t, RecordsDeploymentHistory(t.Context(), false))

ctx := env.Set(t.Context(), DeploymentHistoryVariable, "true")
assert.True(t, RecordsDeploymentHistory(ctx, false))
for _, tc := range []struct {
name string
configured *bool
value string
want bool
}{
{"config true env true", new(true), "true", true},
{"config true env false", new(true), "false", true},
{"config true env unset", new(true), "", true},
{"config false env true", new(false), "true", false},
{"config false env false", new(false), "false", false},
{"config false env unset", new(false), "", false},
{"config unset env true", nil, "true", true},
{"config unset env false", nil, "false", false},
{"config unset env unset", nil, "", false},
} {
t.Run(tc.name, func(t *testing.T) {
ctx := env.Set(t.Context(), DeploymentHistoryVariable, tc.value)
assert.Equal(t, tc.want, RecordsDeploymentHistory(ctx, tc.configured))
})
}
}
5 changes: 4 additions & 1 deletion cmd/bundle/utils/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,10 @@ Run "databricks bundle destroy" first, then deploy again with deployment history
}

func configuresDeploymentHistory(ctx context.Context, b *bundle.Bundle) bool {
configured := b.Config.Experimental != nil && b.Config.Experimental.DeploymentHistory
var configured *bool
if b.Config.Experimental != nil {
configured = b.Config.Experimental.DeploymentHistory
}
return bundleenv.RecordsDeploymentHistory(ctx, configured)
}

Expand Down
4 changes: 2 additions & 2 deletions cmd/bundle/utils/process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ import (

func TestResolveDeploymentHistory(t *testing.T) {
b := &bundle.Bundle{Config: config.Root{
Experimental: &config.Experimental{DeploymentHistory: true},
Experimental: &config.Experimental{DeploymentHistory: new(true)},
}}

newState := &statemgmt.StateDesc{}
assert.True(t, resolveDeploymentHistory(t.Context(), b, newState))
assert.True(t, newState.IsDMS())
assert.False(t, resolveDeploymentHistory(t.Context(), b, &statemgmt.StateDesc{SourcePath: "resources.json"}))
b.Config.Experimental.DeploymentHistory = false
b.Config.Experimental.DeploymentHistory = new(false)
assert.True(t, resolveDeploymentHistory(t.Context(), b, &statemgmt.StateDesc{
SourcePath: "resources.json",
Features: map[string]struct{}{dstate.FeatureDeploymentHistory: {}},
Expand Down
Loading