Ignore EARTHDATA_USERNAME/PASSWORD when EARTHDATA_TOKEN is set - #1122
Conversation
|
I will automatically update this comment whenever this PR is modified
|
|
pre-commit.ci autofix |
0ebeb70 to
3af74a8
Compare
|
@betolink, @mfisher87, @jhkennedy, the integration tests seem to be regularly failing with the following, but re-running them clears things up. It seems odd that we get JSONDecodeErrors that go away on re-runs: |
betolink
left a comment
There was a problem hiding this comment.
Ready to merge just noting that we may need to improve out auth unit test in the future,
| self.token = {"access_token": user_token} | ||
| self.authenticated = True | ||
| if username is not None and password is not None: | ||
| elif username is not None and password is not None: |
There was a problem hiding this comment.
This is a great catch, I don't like these if but we have n different ways of signing in.
| @@ -17,9 +17,15 @@ | |||
|
|
|||
| class TestStoreSessions(unittest.TestCase): | |||
| @responses.activate | |||
There was a problem hiding this comment.
Should we have a test that test both token and EDL credentials are present? other than that I think the code change in auth addresses the core issue.
There was a problem hiding this comment.
Thanks for the review @betolink. Yes, we'll probably want to add more tests to test various combinations of existing/non-existing env vars to cover the various cases to test for interference/precedence. I suspect that this can be covered in the context of work on the other auth issues in the backlog.
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 PRs: #763
example
closes #1. SeeGitHub docs - Linking a pull request to an issue.
CHANGELOG.mdwith details about your change in a section titled## Unreleased. If such a section does not exist, please create one. FollowCommon Changelog for your additions.
Example PRs: #763
README.mdwith 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-supportteam in a comment and wewill help you out!
Request containing "pre-commit.ci autofix" to automate this.
Fixes #1121
📚 Documentation preview 📚: https://earthaccess--1122.org.readthedocs.build/en/1122/