Skip to content

Commit 8d2b6a1

Browse files
committed
lakebox: skip state file writes when nothing changed
setDefault and clearDefault unconditionally rewrote ~/.databricks/ lakebox.json even when the in-memory state was identical to what was already on disk: clearing a profile that wasn't in the map, or re-setting the same value. That created or touched the file for no-op operations. Add change-detection guards to both: setDefault is a no-op when the profile already maps to the requested ID; clearDefault is a no-op when the profile isn't in the map. Result: a CLI invocation that doesn't change state can no longer cause a file to spring into existence on a fresh machine. Tests: - clearDefault on a missing profile must leave the file absent - setDefault with an unchanged value must not bump the mtime - getDefault on a fresh state must not create the file (regression test for the read-only path) Co-authored-by: Isaac
1 parent 59f8216 commit 8d2b6a1

2 files changed

Lines changed: 36 additions & 3 deletions

File tree

cmd/lakebox/state.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ func setDefault(ctx context.Context, profile, lakeboxID string) error {
8181
if err != nil {
8282
return err
8383
}
84+
if state.Defaults[profile] == lakeboxID {
85+
return nil
86+
}
8487
state.Defaults[profile] = lakeboxID
8588
return saveState(ctx, state)
8689
}
@@ -90,6 +93,9 @@ func clearDefault(ctx context.Context, profile string) error {
9093
if err != nil {
9194
return err
9295
}
96+
if _, ok := state.Defaults[profile]; !ok {
97+
return nil
98+
}
9399
delete(state.Defaults, profile)
94100
return saveState(ctx, state)
95101
}

cmd/lakebox/state_test.go

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,36 @@ func TestStateClearDefault(t *testing.T) {
7070
assert.Equal(t, "lakebox-b", getDefault(ctx, "profile-b"))
7171
}
7272

73-
func TestStateClearDefaultMissingProfileNoError(t *testing.T) {
74-
ctx, _ := stateCtx(t)
75-
assert.NoError(t, clearDefault(ctx, "no-such-profile"))
73+
func TestStateClearDefaultMissingProfileDoesNotCreateFile(t *testing.T) {
74+
ctx, path := stateCtx(t)
75+
76+
require.NoError(t, clearDefault(ctx, "no-such-profile"))
77+
78+
_, err := os.Stat(path)
79+
assert.ErrorIs(t, err, fs.ErrNotExist, "clearDefault must not create the state file when there's nothing to remove")
80+
}
81+
82+
func TestStateSetDefaultSameValueDoesNotRewriteFile(t *testing.T) {
83+
ctx, path := stateCtx(t)
84+
85+
require.NoError(t, setDefault(ctx, "profile-a", "lakebox-a"))
86+
before, err := os.Stat(path)
87+
require.NoError(t, err)
88+
89+
// Re-set with the same value should be a no-op.
90+
require.NoError(t, setDefault(ctx, "profile-a", "lakebox-a"))
91+
after, err := os.Stat(path)
92+
require.NoError(t, err)
93+
assert.Equal(t, before.ModTime(), after.ModTime(), "no-op setDefault must not rewrite the file")
94+
}
95+
96+
func TestStateSetDefaultMissingNoFileBeforeWrite(t *testing.T) {
97+
ctx, path := stateCtx(t)
98+
99+
// Loading state on a fresh tempdir must not create the file.
100+
assert.Equal(t, "", getDefault(ctx, "profile-a"))
101+
_, err := os.Stat(path)
102+
assert.ErrorIs(t, err, fs.ErrNotExist, "getDefault must not create the state file")
76103
}
77104

78105
// Pre-existing files from earlier CLI versions carry a `last_profile` field

0 commit comments

Comments
 (0)