Conversation
Introduce libs/atomicfile.Write (same-dir temp + chmod + rename) and route the hand-rolled atomic writers and a few plain os.WriteFile state saves through it. The result always gets the caller-specified mode; it does not inherit the replaced file's permissions. Pure refactor (already atomic, behavior unchanged): - bundle/direct/dstate/state.go (deployment resources.json) - libs/auth/storage/filestore.go (token cache) - libs/aitools/installer/state.go - libs/localenv/constraints.go - libs/clicompat/clicompat.go - experimental/ssh/internal/sshconfig/knownhosts.go - cmd/genie/conversations.go (best-effort store; also drops a double-close) Bug fixes: - libs/cache/file_cache.go: removed the os.Remove-before-rename hack, which deleted the destination first and defeated atomicity (os.Rename already replaces an existing file on Windows). - cmd/sandbox/state.go, cmd/sandbox/sshconfig.go: used a fixed "<path>.tmp" temp name, so concurrent CLI runs collided on it; CreateTemp avoids that. New atomicity (was a plain os.WriteFile): - bundle/statemgmt/state_pull.go: the state file pulled from the remote is now written atomically (closes the "TODO: write + rename"). Co-authored-by: Isaac <no-reply@databricks.com>
Approval status: pending
|
Co-authored-by: Isaac <no-reply@databricks.com>
Integration test reportCommit: 0adf30c
6 interesting tests: 6 flaky
Top 3 slowest tests (at least 2 minutes):
|
Add a functional-options MkDir(perm) to atomicfile.Write so a caller can create the parent directory (with its own deliberate mode: 0o700 for secrets, 0o755 otherwise) in the same call. Route the eight sites that did an explicit MkdirAll right before the write through it, and inline localenv.writeCacheAtomic, which became a single-caller pass-through. Co-authored-by: Isaac <no-reply@databricks.com>
Route the remaining in-place os.WriteFile saves that overwrite a persistent
file (torn write would lose the prior content) through atomicfile.Write:
New atomicity for files a user owns:
- bundle/configsync/output.go: rewriting local source config files on sync
- libs/completion/{install,uninstall}.go: editing the user's shell RC file
(mode preserved via the existing info.Mode()); install now rewrites instead
of appending, matching uninstall
- libs/localenv/pipeline.go: overwriting pyproject.toml on merge
New atomicity for CLI-owned state/cache read back later:
- cmd/labs/project/{login,project}.go: labs auth and version files
- cmd/labs/localcache/jsonfile.go: labs JSON cache (write+mkdir+retry collapsed
into one MkDir call)
- bundle/configsync/diff.go: local config snapshot used for the next diff
Left as-is: the aitools installer writes to a temp staging dir already promoted
by rename, .databrickscfg (go-ini SaveTo, needs its own approach), and the
experimental/ssh in-place config editors.
Co-authored-by: Isaac <no-reply@databricks.com>
Remove atomicWriteFile and installFish, which became single-expression pass-throughs to atomicfile.Write, inlining them into their callers. Co-authored-by: Isaac <no-reply@databricks.com>
2 participants
Adds
libs/atomicfile.Write(same-directory temp file + chmod +os.Rename) and routes in-place file saves through it, so a crash or a concurrent reader never sees a partial file. The written file always gets the caller-specified mode and does not inherit the replaced file's permissions. An optionalatomicfile.MkDir(perm)creates the parent directory in the same call (the directory mode stays a deliberate per-caller choice: 0o700 for secrets, 0o755 otherwise).Pure refactor (already atomic, behavior unchanged)
bundle/direct/dstate/state.go— deploymentresources.jsonlibs/auth/storage/filestore.go— token cachelibs/aitools/installer/state.golibs/localenv/constraints.go(inlined the now-thinwriteCacheAtomic)libs/clicompat/clicompat.goexperimental/ssh/internal/sshconfig/knownhosts.gocmd/genie/conversations.go— best-effort store (also drops a double-close)Bug fixes
libs/cache/file_cache.go— removed theos.Remove-before-Renamehack that deleted the destination first and defeated atomicity.os.Renamealready replaces an existing file on Windows.cmd/sandbox/state.go,cmd/sandbox/sshconfig.go— used a fixed<path>.tmptemp name, so concurrent CLI runs collided on it.CreateTempavoids that.New atomicity (was a plain
os.WriteFile)Files a user owns (a torn write would lose their content):
bundle/configsync/output.go— local source config files rewritten on synclibs/completion/{install,uninstall}.go— the user's shell RC file; mode preserved via the existinginfo.Mode().installnow rewrites the file instead ofO_APPEND, matchinguninstall.libs/localenv/pipeline.go—pyproject.tomlon mergeCLI-owned state/cache read back on the next run:
bundle/statemgmt/state_pull.go— state pulled from the remote (closes the// TODO: write + rename)cmd/labs/project/{login,project}.go— labs auth and version filescmd/labs/localcache/jsonfile.go— labs JSON cachebundle/configsync/diff.go— local config snapshot used for the next diffDeliberately left out
libs/databrickscfg/ops.go— writes via go-iniSaveToin place; needs a render-to-temp approach, its own PRexperimental/sshin-place~/.ssh/configand VSCode settings editors — experimental, higher-risk read-modify-writelibs/aitools/installerfile writes — staged in a temp dir already promoted by rename, so the atomicity boundary is the directoryThis pull request and its description were written by Isaac.