Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic success check to write_file() and use atomic file-replacing function #161

Merged
merged 2 commits into from
Dec 17, 2023

Conversation

hanzi
Copy link
Collaborator

@hanzi hanzi commented Dec 17, 2023

This checks whether write() returns the correct number of characters written, and uses os.replace() to replace the file (which is supposed to be atomic) rather than deleting the old file and then renaming the new one.

This may or may not help with instances of file corruption what we have seen. Since it's difficult to reproduce the issue, it's hard to be sure about that.

But at the very least it shouldn't make things worse.

…ng function

This checks whether `write()` returns the correct number of characters written, and uses `os.replace()` to replace the file (which is supposed to be atomic) rather than deleting the old file and _then_ renaming the new one.

This may or may not help with instances of file corruption what we have seen. Since it's difficult to reproduce the issue, it's hard to be sure about that.

But at the very least it shouldn't make things worse.
@hanzi hanzi requested a review from 40Cakes December 17, 2023 13:09
@40Cakes 40Cakes merged commit 7aff90c into 40Cakes:main Dec 17, 2023
1 check passed
@hanzi hanzi deleted the error-checking-in-write-file branch January 7, 2024 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants