Skip to content
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

Move tokenstorage #1065

Merged
merged 4 commits into from
Sep 27, 2024
Merged

Move tokenstorage #1065

merged 4 commits into from
Sep 27, 2024

Conversation

derek-globus
Copy link
Contributor

@derek-globus derek-globus commented Sep 27, 2024

To help with reviewing, each of the below steps have been done in separate commits.

What?

  1. Move TokenStorage and its subclasses from globus_sdk.experimental.tokenstorage to globus_sdk.tokenstorage (with respect to terms of where it's exposed).
  2. Move the existing docs/tokenstorage.rst doc to docs/authorization/token_caching/storage_adapters.rst, leaving behind an orphaned page redirecting users with existing links.
  3. Update docstrings and create the docs/authorization/token_caching/token_storage.rst page.

📚 Documentation preview 📚: https://globus-sdk-python--1065.org.readthedocs.build/en/1065/

@derek-globus
Copy link
Contributor Author

Internal Tracking

https://app.shortcut.com/globus/story/35753

@@ -0,0 +1,20 @@

Token Caching
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe controversially I titled this section "token caching" rather than "token storage". This is to disambiguate with the subpages that provide references for specific versions of the same concept.

Alternative Structure

docs/
  - authorization/
     - token_storage/
         - token_storages.rst
         - index.rst
         - storage_adapters.rst

and you'd end up with pages titled "Token Storage", "Token Storages", and "Storage Adapters (Legacy)".

Copy link
Member

@kurtmckee kurtmckee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I acknowledge that there is quite a bit of docstring pedantry here.

Nevertheless, I think that it will increase internal consistency, and make the documentation more readable.

docs/authorization/token_caching/index.rst Outdated Show resolved Hide resolved
docs/authorization/token_caching/storage_adapters.rst Outdated Show resolved Hide resolved
docs/authorization/token_caching/token_storages.rst Outdated Show resolved Hide resolved
src/globus_sdk/tokenstorage/v2/base.py Outdated Show resolved Hide resolved
src/globus_sdk/tokenstorage/v2/base.py Outdated Show resolved Hide resolved
src/globus_sdk/tokenstorage/v2/sqlite.py Outdated Show resolved Hide resolved
src/globus_sdk/tokenstorage/v2/token_data.py Outdated Show resolved Hide resolved
src/globus_sdk/tokenstorage/v2/token_data.py Outdated Show resolved Hide resolved
src/globus_sdk/tokenstorage/v2/token_data.py Outdated Show resolved Hide resolved
src/globus_sdk/tokenstorage/v2/token_data.py Outdated Show resolved Hide resolved
@derek-globus derek-globus merged commit e3dd804 into globus:main Sep 27, 2024
16 checks passed
@derek-globus derek-globus deleted the move-tokenstorage branch September 27, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants