fix: distinguish between wrong and no earthdata credentials - #162
Merged
Merged
Conversation
A username/password earthdata secret that Earthdata Login refused surfaced as "failed to establish an Earthdata Login identity" on the first request, and every request in the following 60s backoff window fell through to earthaccess-auth's generic "no non-interactive EDL login strategy available: set EARTHDATA_TOKEN ..." message, which reads as if no credentials were configured at all. - Catch LoginAttemptFailure separately when logging in with the secret and say that Earthdata Login did not accept the secret's EARTHDATA_USERNAME/EARTHDATA_PASSWORD. EDL's response body still goes to the service log only. - Remember a failed first load's sanitized message and re-raise it for requests inside its backoff window, so a secret that is missing keys, unreadable, or rejected keeps reporting that specific cause. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
maxrjones
changed the title
The module docstring and _secret_arn_unless_latched described the rule in one dash-laden sentence that was hard to parse. Replace it with the three cases, in order: secret loads, secret unreachable, no ARN configured. State why the secret wins even when the environment already holds a working identity: otherwise the secret is never read and rotation has no effect until a restart. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The docstring described first-load and refresh failures in two long sentences joined by dashes, and left out the case where a failed first fetch falls back to an identity already in the environment. Split it into the three failure cases the code actually handles and state the fallback explicitly. Reword the Raises entry and the nothing-to-fall- back-to comment in _on_fetch_failure to match. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
hrodmn
approved these changes
Sep 30, 2026
hrodmn
left a comment
hrodmn
left a comment
Contributor
There was a problem hiding this comment.
Thanks for adding the additional guardrails here - I am looking forward to the day where we can delegate EDL credential management to an external service!
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
2 participants
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.
Summary
This PR makes the failed login error surface on all requests that fail during a given 60s backoff window, rather than falling back to a generic unavailable login strategy.
This fixes an issue that surfaced in VEDA testing and made debugging more challenging than necessary.
Testing
PR checks
run-cdk-checkslabel to this PR.deploy-devlabel. It smoke-tests tiles from the native MUR, virtual MUR, and virtual NLDAS Icechunk stores after deployment.