Skip to content

Commit 2d157c0

Browse files
Enable caching user identity by default (#4202)
## Changes Enable caching user identity by default ## Why Dry-run was successful and metrics show that it can save several seconds in a deploy time ## Tests Existing tests pass <!-- If your PR needs to be included in the release notes for next release, add a separate entry in NEXT_CHANGELOG.md as part of your PR. --> --------- Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
1 parent 2cb6940 commit 2d157c0

10 files changed

Lines changed: 168 additions & 20 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@
33
## Release v0.283.0
44

55
### Notable Changes
6+
* Bundle commands now cache the user's account details to improve command latency.
7+
To disable this, set the environment variable DATABRICKS_CACHE_ENABLED to false.
68

79
### CLI
810

911
### Bundles
12+
* Enable caching user identity by default ([#4202](https://github.com/databricks/cli/pull/4202))
1013

1114
### Dependency updates
1215

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
[Env]
2+
DATABRICKS_CACHE_ENABLED = 'false'
3+
14
[[Repls]]
25
Old = 'terraform.tfstate'
36
New = 'resources.json'
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
Badness = "In TF error message talks about ingestion_definition being a list which is not the case. In direct error message says 'on invalid' which is confusing, should say about ingestion_definition being nil"
22

3+
[Env]
4+
DATABRICKS_CACHE_ENABLED = 'false'
5+
36
[[Repls]]
47
Old = 'terraform.tfstate'
58
New = 'resources.json'

acceptance/cache/clear/test.toml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
Cloud = false
22
Local = true
33

4-
[Env]
5-
DATABRICKS_CACHE_ENABLED = 'true'
6-
74
# Redact structured logging fields from debug output
85
[[Repls]]
96
Old = ' pid=[0-9]+'

acceptance/cache/simple/test.toml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,6 @@ Local = true
33

44
RecordRequests = true
55

6-
[Env]
7-
DATABRICKS_CACHE_ENABLED = 'true'
8-
96
# Redact structured logging fields from debug output
107
[[Repls]]
118
Old = ' pid=[0-9]+'

bundle/bundle.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ type Bundle struct {
151151
Tagging tags.Cloud
152152

153153
// Cache is used for caching API responses (e.g., current user).
154-
// By default, operates in measurement-only mode. Set DATABRICKS_CACHE_ENABLED=true to enable actual caching.
154+
// By default, caching is enabled. Set DATABRICKS_CACHE_ENABLED=false to disable caching.
155155
Cache *cache.Cache
156156

157157
Metrics Metrics

libs/cache/file_cache.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,12 @@ func getCacheBaseDir(ctx context.Context) (string, error) {
109109

110110
// NewCache creates a new file-based cache using UserCacheDir() + "databricks" + version + cached component name.
111111
// Including the CLI version in the path ensures cache isolation across different CLI versions.
112-
// By default, the cache operates in measurement-only mode (cacheEnabled=false), which means it will:
112+
// By default, caching is enabled. Set DATABRICKS_CACHE_ENABLED to false/0/no/off to disable caching.
113+
// When caching is disabled (measurement-only mode), the cache will:
113114
// - Check if cached values exist
114115
// - Measure how much time would have been saved
115116
// - Emit metrics about potential savings
116117
// - Always compute the value (never actually use the cache)
117-
// Set DATABRICKS_CACHE_ENABLED=true to enable actual caching.
118118
// The returned cache can handle multiple types through the generic GetOrCompute function.
119119
func NewCache(ctx context.Context, component string, expiry time.Duration, metrics Metrics) *Cache {
120120
cacheBaseDir, err := getCacheBaseDir(ctx)
@@ -132,9 +132,14 @@ func NewCache(ctx context.Context, component string, expiry time.Duration, metri
132132
}
133133
fc.metrics = metrics
134134

135-
// Check if cache is enabled; default is false (measurement-only mode)
136-
// Only "true" enables caching; any other value (including "false", "1", etc.) keeps it disabled
137-
fc.cacheEnabled = env.Get(ctx, "DATABRICKS_CACHE_ENABLED") == "true"
135+
// Check if cache is enabled
136+
// Default is true (enabled) when DATABRICKS_CACHE_ENABLED is not set
137+
// When set, parse it as a boolean
138+
if enabled, ok := env.GetBool(ctx, "DATABRICKS_CACHE_ENABLED"); ok {
139+
fc.cacheEnabled = enabled
140+
} else {
141+
fc.cacheEnabled = true
142+
}
138143
return &Cache{impl: fc}
139144
}
140145

libs/cache/file_cache_env_test.go

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package cache
22

33
import (
44
"context"
5+
"fmt"
56
"os"
67
"path/filepath"
78
"runtime"
@@ -22,43 +23,92 @@ func TestCacheEnabledEnvVar(t *testing.T) {
2223
tests := []struct {
2324
name string
2425
envValue string
26+
setEnv bool
2527
expectCached bool
2628
}{
2729
{
2830
name: "cache enabled with 'true'",
2931
envValue: "true",
32+
setEnv: true,
33+
expectCached: true,
34+
},
35+
{
36+
name: "cache enabled with 'TRUE'",
37+
envValue: "TRUE",
38+
setEnv: true,
39+
expectCached: true,
40+
},
41+
{
42+
name: "cache enabled with '1'",
43+
envValue: "1",
44+
setEnv: true,
45+
expectCached: true,
46+
},
47+
{
48+
name: "cache enabled with 'yes'",
49+
envValue: "yes",
50+
setEnv: true,
51+
expectCached: true,
52+
},
53+
{
54+
name: "cache enabled with 'on'",
55+
envValue: "on",
56+
setEnv: true,
3057
expectCached: true,
3158
},
3259
{
3360
name: "cache disabled with 'false'",
3461
envValue: "false",
62+
setEnv: true,
3563
expectCached: false,
3664
},
3765
{
38-
name: "cache disabled when empty",
66+
name: "cache disabled with 'FALSE'",
67+
envValue: "FALSE",
68+
setEnv: true,
69+
expectCached: false,
70+
},
71+
{
72+
name: "cache disabled with '0'",
73+
envValue: "0",
74+
setEnv: true,
75+
expectCached: false,
76+
},
77+
{
78+
name: "cache disabled with 'no'",
79+
envValue: "no",
80+
setEnv: true,
81+
expectCached: false,
82+
},
83+
{
84+
name: "cache disabled with empty string",
3985
envValue: "",
86+
setEnv: true,
4087
expectCached: false,
4188
},
4289
{
4390
name: "cache disabled with invalid value",
44-
envValue: "yes",
91+
envValue: "invalid",
92+
setEnv: true,
4593
expectCached: false,
4694
},
4795
{
48-
name: "cache disabled with '1'",
49-
envValue: "1",
50-
expectCached: false,
96+
name: "cache enabled by default when not set",
97+
envValue: "",
98+
setEnv: false,
99+
expectCached: true,
51100
},
52101
}
53102

54-
for _, tt := range tests {
103+
for i, tt := range tests {
55104
t.Run(tt.name, func(t *testing.T) {
56105
// Create a unique subdirectory for this test
57-
testDir := filepath.Join(tempDir, tt.name)
106+
// Use index to avoid case-sensitivity issues on macOS
107+
testDir := filepath.Join(tempDir, fmt.Sprintf("test-%d-%s", i, tt.envValue))
58108

59109
// Set up context with environment variable
60110
testCtx := ctx
61-
if tt.envValue != "" {
111+
if tt.setEnv {
62112
testCtx = env.Set(testCtx, "DATABRICKS_CACHE_ENABLED", tt.envValue)
63113
}
64114
testCtx = env.Set(testCtx, "DATABRICKS_CACHE_DIR", testDir)

libs/env/context.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"os"
77
"runtime"
8+
"strconv"
89
"strings"
910
)
1011

@@ -55,6 +56,41 @@ func Get(ctx context.Context, key string) string {
5556
return v
5657
}
5758

59+
// GetBool gets a boolean value from the context or environment.
60+
// Returns (value, true) if the key is set, or (false, false) if not set.
61+
// It accepts various boolean-like values:
62+
// - True: "1", "t", "T", "true", "TRUE", "True", "yes", "YES", "Yes", "on", "ON", "On"
63+
// - False: "0", "f", "F", "false", "FALSE", "False", "no", "NO", "No", "off", "OFF", "Off", "" (empty string)
64+
// Invalid values are treated as false but still return ok=true.
65+
func GetBool(ctx context.Context, key string) (bool, bool) {
66+
v, ok := Lookup(ctx, key)
67+
if !ok {
68+
return false, false
69+
}
70+
71+
// Empty string is treated as false
72+
if v == "" {
73+
return false, true
74+
}
75+
76+
// Handle additional boolean-like values not covered by strconv.ParseBool
77+
switch strings.ToLower(v) {
78+
case "yes", "on":
79+
return true, true
80+
case "no", "off":
81+
return false, true
82+
}
83+
84+
// Use strconv.ParseBool for standard boolean parsing
85+
// It handles: "1", "t", "T", "true", "TRUE", "True" (true)
86+
// and: "0", "f", "F", "false", "FALSE", "False" (false)
87+
b, err := strconv.ParseBool(v)
88+
if err != nil {
89+
return false, true
90+
}
91+
return b, true
92+
}
93+
5894
// Set key on the context.
5995
//
6096
// Note: this does NOT mutate the processes' actual environment variables.

libs/env/context_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,57 @@ func TestHome(t *testing.T) {
5555
assert.Equal(t, "...", home)
5656
assert.NoError(t, err)
5757
}
58+
59+
func TestGetBool(t *testing.T) {
60+
testutil.CleanupEnvironment(t)
61+
ctx := context.Background()
62+
63+
// Test true values
64+
trueValues := []string{"true", "TRUE", "True", "1", "t", "T", "yes", "YES", "Yes", "on", "ON", "On"}
65+
for _, v := range trueValues {
66+
t.Run("true_"+v, func(t *testing.T) {
67+
ctx := Set(ctx, "TEST_BOOL", v)
68+
val, ok := GetBool(ctx, "TEST_BOOL")
69+
assert.True(t, ok, "expected key to be set")
70+
assert.True(t, val, "expected %q to be true", v)
71+
})
72+
}
73+
74+
// Test false values
75+
falseValues := []string{"false", "FALSE", "False", "0", "f", "F", "no", "NO", "No", "off", "OFF", "Off", ""}
76+
for _, v := range falseValues {
77+
t.Run("false_"+v, func(t *testing.T) {
78+
ctx := Set(ctx, "TEST_BOOL", v)
79+
val, ok := GetBool(ctx, "TEST_BOOL")
80+
assert.True(t, ok, "expected key to be set")
81+
assert.False(t, val, "expected %q to be false", v)
82+
})
83+
}
84+
85+
// Test invalid/unknown values default to false but ok=true
86+
invalidValues := []string{"invalid", "random", "2", "maybe"}
87+
for _, v := range invalidValues {
88+
t.Run("invalid_"+v, func(t *testing.T) {
89+
ctx := Set(ctx, "TEST_BOOL", v)
90+
val, ok := GetBool(ctx, "TEST_BOOL")
91+
assert.True(t, ok, "expected key to be set")
92+
assert.False(t, val, "expected %q to be false (invalid)", v)
93+
})
94+
}
95+
96+
// Test missing key returns ok=false
97+
val, ok := GetBool(ctx, "NON_EXISTENT_KEY")
98+
assert.False(t, ok, "expected key to not be set")
99+
assert.False(t, val, "expected value to be false when not set")
100+
101+
// Test from actual environment variable
102+
t.Setenv("TEST_ENV_BOOL", "true")
103+
val, ok = GetBool(context.Background(), "TEST_ENV_BOOL")
104+
assert.True(t, ok)
105+
assert.True(t, val)
106+
107+
t.Setenv("TEST_ENV_BOOL_FALSE", "0")
108+
val, ok = GetBool(context.Background(), "TEST_ENV_BOOL_FALSE")
109+
assert.True(t, ok)
110+
assert.False(t, val)
111+
}

0 commit comments

Comments
 (0)