Skip to content

Prevent interrupted FNL downloads leaving truncated files - #87

Merged
aethr merged 3 commits into
mainfrom
fix-partial-fnl-downloads
Oct 11, 2026
Merged

aethr merged 3 commits into
mainfrom
fix-partial-fnl-downloads

Conversation

@aethr

@aethr aethr commented Oct 8, 2026

Copy link
Copy Markdown

Description

Fixes #81.

The upstream FNL file for 2022-11-01 00Z is intact: a fresh download subsets cleanly with wgrib2. The failure was caused by a truncated local copy:

  • download_file wrote directly to the final filename. Cleanup only ran on a RequestException, so a download that was killed part way (container stopped, Ctrl-C, joblib terminating workers after another download failed) left a partial file in place.
  • setup_for_wrf.py only checks whether an FNL file exists before skipping its download, so subsequent runs reused the truncated file and wgrib2 failed with read outside of file, bad grib file.

Six truncated files were found in a local FNL cache, each roughly 35–50% of the expected size.

This PR downloads to a <name>.part file, verifies its size against Content-Length (when the response isn't content-encoded), and then moves it into place with os.replace. The .part file is always removed on failure or interruption, so the final path only ever holds a complete file.

Checklist

Please confirm that this pull request has done the following:

  • Tests added
  • Documentation added (where applicable)
  • Changelog item added to changelog/

Notes

Existing truncated files in FNL caches are not detected by this change and need to be deleted manually.

🤖 Generated with Claude Code

aethr and others added 2 commits October 8, 2026 16:24
Downloads were written directly to their final path, so a download that
was killed part way through left a truncated file behind. Subsequent runs
treat existing files as complete, causing wgrib2 to fail with "read
outside of file, bad grib file".

Download to a .part file, verify its size against Content-Length, then
atomically move it into place.

Fixes #81

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@aethr aethr self-assigned this Oct 8, 2026
@aethr
aethr requested a review from prayner October 8, 2026 05:29
@prayner

prayner commented Oct 10, 2026

Copy link
Copy Markdown

test_fetch_fnl.py:29 why testing for an empty list rather than a null
path?

@aethr

aethr commented Oct 10, 2026

Copy link
Copy Markdown
Author

test_fetch_fnl.py:29 why testing for an empty list rather than a null path?

Stylistically, there is consistency with the initial test, and all the following tests:

# expected file exists
assert list(tmp_path.iterdir()) == [result]

# no file exists
assert list(tmp_path.iterdir()) == []

In the case of the tests where no file is expected, this check covers a lot of scenarios:

  • the file wasn't downloaded to the expected filename
  • the file wasn't downloaded to any filename
  • a partial file isn't present

I don't mind also adding the explicit "expected file doesn't exist" assertion if you think it makes it clearer, or adding a comment to these tests.

@aethr

aethr commented Oct 11, 2026

Copy link
Copy Markdown
Author

Added better comments to the assertions here.

@aethr
aethr merged commit 6005960 into main Oct 11, 2026
7 checks passed
@aethr
aethr deleted the fix-partial-fnl-downloads branch October 11, 2026 03:42
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.

grib subsetting fails on au-test domain on 2022-10-31

2 participants