Repository navigation
fix: preserve ledgers when formatter writes fail - #1054
Open
ryanduguid wants to merge 2 commits into
Open
ryanduguid wants to merge 2 commits into
ryanduguid wants to merge 2 commits into
Conversation
A new output file has nothing to preserve, so write it directly instead of through a 0600 temporary file. Flush and fsync replacement files before the rename.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1051.
bean-format --in-placeandbean-format input -o inputcan truncate the ledger before a failing write completes. A regression that writes 10 characters and then raises an error reproduces the data loss in both paths.Write regular-file outputs to a temporary file in the destination directory, close it, then replace the destination. Write, close and replacement failures preserve the original and clean up the temporary file. Open input files only while reading so replacing the same path also works on Windows. Standard input/output and device outputs remain supported; in-place standard input is rejected.
Tests cover both output paths, partial-write and close failures, failed replacement, multiple files, standard streams, read-only inputs, symlinks and POSIX permission bits.
Validation on Windows, Python 3.12.10:
pytest beancount -q -o faulthandler_timeout=30: 1,118 passed, 24 skipped, 2 expected failures.meson test -C build-test: one native test passed.mypy .: no issues in 159 source files, with a warning that current Mypy no longer supports the configured Python 3.9 target.git diff --checkpass, except the pinned Mypy hook. That hook fails parsing the installed Click dependency under the Python 3.9 configuration; the same failure occurs on the unmodified upstream formatter.The two POSIX permission tests were skipped locally. The full OS/Python CI matrix and repository-wide pre-commit run remain unverified. Atomic replacement follows symlinks and preserves permission bits, but other hard links retain the old contents and other file metadata may change, as documented in the CLI help. Replacing a file also needs write access to its directory for the temporary file; a writable ledger in a read-only directory now fails before the ledger is changed. This does not claim durability across power loss.