-
Notifications
You must be signed in to change notification settings - Fork 10
Update auth tests #397
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
Update auth tests #397
Conversation
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.
TODO:
- Run full test suite, including the new interactive
globus_auth.bash, on Chrysalis, Perlmutter, Compy
| cache=zstash # Set via `self.cache = "zstash"` | ||
|
|
||
| # Start fresh by deleting token file: | ||
| rm ~/.zstash_globus_tokens.json |
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.
This is the fix for the issue noted in the E3SM Unified 1.12.0rc2 error
| echo "Test 1: What if we switch to a different endpoint? #####################" | ||
| test_different_endpoint1 ${path_to_repo} NERSC_PERLMUTTER_ENDPOINT ${perlmutter_dst_dir} | ||
| echo "Test 2: What if we try a) revoking consents and then b) removing the token file? ###" | ||
| test_different_endpoint2 ${path_to_repo} NERSC_PERLMUTTER_ENDPOINT ${perlmutter_dst_dir} | ||
| echo "Test 3: What if we switch to a different endpoint again, but first remove the token file? ###" | ||
| test_different_endpoint3 ${path_to_repo} NERSC_HPSS_ENDPOINT ${hpss_dst_dir} |
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.
@chengzhuzhang @golaz A few notes on zstash behavior:
Case 1: no external changes
test_different_endpoint1
- Use zstash with destination endpoint A (accomplished in the test by previously running
test_single_auth_code) - Use zstash with destination endpoint B
- Fails with
TransferAPIError
Case 2: revoke consents then remove token file
test_different_endpoint2
- Use zstash with destination endpoint A (accomplished in the test by previously running
test_different_endpoint1) - Revoke consents
- Use zstash with destination endpoint B
- Fails with
AuthAPIError'Bad Request'(This is the source of the error encountered in Unified testing) - Remove the token file.
- Try again: zstash with destination endpoint B
- Success
Case 3: Just remove the token file
test_different_endpoint3
- Use zstash with destination endpoint A (accomplished in the test by previously running
test_different_endpoint2) - Remove the token file (This is the fix to the error encountered in Unified testing)
- Use zstash with destination endpoint B
- Success
Conclusion
As you can see, case 3 is the simplest successful solution.
Good: It means that we can have multiple Globus consents simultaneously registered at https://auth.globus.org/v2/web/consents > Globus Endpoint Performance Monitoring, perhaps associated with different Globus endpoints.
Possible future improvements: At the moment, zstash can only support one saved local token at a time. That means if you're using zstash on multiple destination endpoints, you'll have to re-authenticate every time you switch. I think this should be possible to change in the future, but I imagine it is out of scope for the Unified testing period. Please let me know if that should be priortized.
|
The fix for the failed test looks fine to me. The test uses ~/.zstash_globus_tokens.json and currently deletes it, it might be safer to use a tmp directory for storing the token for the test, but this can be addressed later. |
Addressed in Point 1 of #398.
Addressed in Point 2 of #398. |
The GitHub Actions are passing, so merging. |
Summary
Objectives:
Select one: This pull request is...
Small Change