-
Notifications
You must be signed in to change notification settings - Fork 96
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
Change auth strategy to using find_or_create_token endpoint #782
Conversation
Oh, didn't have those errors locally and will check what went wrong there. Edit: I only ran the integration tests -.- |
616ea5d
to
37b2370
Compare
I am not sure about this and could just initialize with an empty dict to make |
@fwfichtner, you can safely ignore the failing integration tests. They are all tests that will pass once the PR is merged. We still don't know why this is happening, but we've seen it a few recent PRs. Regarding the failing unit tests, I'll comment in the code. |
review suggestions Co-authored-by: Chuck Daniels <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fwfichtner, looks good! Thank you!
@mfisher87 and/or @betolink, either of you care to take a quick look at this PR before I merge it? |
I'm not going to be able to take a close look at this in the short term. Funding & time-management struggles 😩 |
Thanks for the review @chuckwondo |
@chuckwondo thanks for merging! fyi I just got my funding confirmed yesterday; it's not much, but I'm out of "am I funded for this at all?" space now :) |
Like discussed in in #484 this simplifies authentication by using the
find_or_create_token
-endpoint and removing all other auth-endpoints. If all is good users should not notice this change besides when their tokens expire, then they should now just be re-created (I couldn't test that because I had no expired tokens available for testing that).auth.refresh_token()
has been deprecated.Closes #484
Fixes #764
Pull Request (PR) draft checklist - click to expand
contributing documentation
before getting started.
title such as "Add testing details to the contributor section of the README".
example
closes #1
. SeeGitHub docs - Linking a pull request to an issue.
CHANGELOG.md
with details about your change in a section titled## Unreleased
. If such a section does not exist, please create one. FollowCommon Changelog for your additions.
README.md
with details of changes to theearthaccess interface, if any. Consider new environment variables, function names,
decorators, etc.
Click the "Ready for review" button at the bottom of the "Conversation" tab in GitHub
once these requirements are fulfilled. Don't worry if you see any test failures in
GitHub at this point!
Pull Request (PR) merge checklist - click to expand
Please do your best to complete these requirements! If you need help with any of these
requirements, you can ping the
@nsidc/earthaccess-support
team in a comment and wewill help you out!
Request containing "pre-commit.ci autofix" to automate this.
📚 Documentation preview 📚: https://earthaccess--782.org.readthedocs.build/en/782/