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

Catch error in write to remove broken file #483

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

felixcremer
Copy link
Contributor

When the write command errors it still sometimes construct a broken file on disk.
This change should make sure, that the write function would tidy up these broken files after faliure, because they are in the way for later analysis.

Do you think, that this is enough to capture all broken files?
Should this also be tested for other backends?

Locally I get some spurious failures which have nothing to do with the changes but are coming from some download time out errors.

@codecov-commenter
Copy link

Codecov Report

Merging #483 (17c4cac) into main (2603bd1) will decrease coverage by 0.17%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #483      +/-   ##
==========================================
- Coverage   81.15%   80.98%   -0.17%     
==========================================
  Files          49       49              
  Lines        4113     4108       -5     
==========================================
- Hits         3338     3327      -11     
- Misses        775      781       +6     
Files Changed Coverage Δ
src/write.jl 89.79% <100.00%> (+0.90%) ⬆️

... and 7 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@rafaqz
Copy link
Owner

rafaqz commented Jul 27, 2023

I think we need to first check the file didn't already exist!

One error could be that force=true was not used and the file existed.

Edit: thinking about it, this gets a little tricky if we use force=true and error before we change the original... maybe we should always write to a temp file, and only rename it when everything has worked? otherwise deleting it? Not sure...

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.

3 participants