Exit non-zero on a failed download, and pin fetchez to a version with the auth fix - #100
Merged
Conversation
Earlier fetchez releases injected the Earthdata credentials into the initial Harmony request headers, and the redirect to Earthdata Login dropped them. Earthdata served its HTML login page instead, and the download died parsing that page as JSON. fetchez 0.6.13 is the first release carrying the redirect-safe requests.Session that fixes it. The dependency was unpinned, so a fresh install could still resolve a version with the bug.
A Harmony request that came back with nothing logged a warning, skipped that sub-region and moved on to the next, and 'ivert database download' still exited 0. A download that retrieved no data at all was therefore indistinguishable from a successful one, and a script driving IVERT had no way to tell. download_new_granules() now returns a DownloadSummary counting how each sub-region of the request turned out, and the CLI raises a ClickException when any part failed. Only a Harmony request that returned nothing counts as a failure. A sub-region Harmony served correctly that added no new granules -- it holds no ICESat-2 data, or every granule over it is already in the database -- is counted separately and is still a success. Whatever did download is saved, and re-running the command retries only the parts still missing.
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.
Two loose ends from #23, which is otherwise resolved between fetchez's
redirect fix and IVERT's
.netrchandling (#46, #74).Pin
fetchezto >= 0.6.13The
JSONDecodeErrorin #23 came from fetchez injecting the Earthdatacredentials into the initial Harmony request headers: the redirect to
Earthdata Login dropped them, Earthdata served its HTML login page, and
parsing that page as JSON blew up. matth-love fixed this in
continuous-dems/fetchez#277 with a redirect-safe
requests.Session,first released in fetchez v0.6.13.
IVERT listed
fetchezunpinned, so a fresh install could still resolve aversion with the bug and reproduce the original report.
Exit non-zero when part of a download fails
The last open comment on #23: the error was reported, then "skipping"
logic ran and the program exited 0.
A Harmony request that came back with nothing logged a warning, skipped
that sub-region and moved on, and
ivert database downloadstill exited0 — so a download that retrieved nothing at all looked exactly like a
successful one, and a script driving IVERT had no way to tell.
download_new_granules()now returns aDownloadSummarycounting howeach sub-region turned out, and the CLI raises a
ClickExceptionwhenany part failed.
The distinction that matters: only a Harmony request that returned
nothing counts as a failure. A sub-region Harmony served correctly
that added no new granules — the region holds no ICESat-2 data, or every
granule over it is already in the database — is counted separately and is
still a success. Errors are still counted rather than raised inside the
loop, so one unreachable sub-region does not abandon the rest of the
request; whatever did download is saved, and re-running retries only the
parts still missing.
Partial failure also exits non-zero, since a failed part leaves a hole in
the region that was asked for.
Testing
Exercised through the real CLI path with a stubbed
IS2Database:logger.errorprints at every verbosity levelAn AST pass over
download_new_granulesconfirms no barereturnremains, so the CLI cannot receive
None.prek run --all-filespasses.Follow-up
The conda-forge feedstock recipe still lists
fetchezunpinned and needsthe same
>= 0.6.13bound, or a conda install keeps the hole this closes.🔍 Docs preview: https://ivert--100.org.readthedocs.build/en/100/