Skip to content

Update DynamoDB TokenStorage example + mypy config - #1421

Merged
sirosen merged 2 commits into
globus:mainfrom
sirosen:update-dynamodb-example
Aug 14, 2026
Merged

Update DynamoDB TokenStorage example + mypy config#1421
sirosen merged 2 commits into
globus:mainfrom
sirosen:update-dynamodb-example

Conversation

@sirosen

@sirosen sirosen commented Aug 14, 2026

Copy link
Copy Markdown
Member

In order to make the DynamoDB token storage example pass mypy linting and
use the current paradigm for TokenStorage, it needs:

  • missing annotations (easy)
  • retrofitting of existing functions (easy)
  • mypy overrides to ignore boto3 types (easy, but note below)
  • an implementation of get_token_data_by_resource_server (see note!)

For mypy, I have decided to move to a dedicated mypy.ini config in the
docs/ directory. With the config growing, it didn't make sense to keep
it embedded in the repo root pyproject.toml . That also lets us set
different top-level behavioral flags in the config.

For get_token_data_by_resource_server, I decided to implement it, in
the simplest possible way using a sequential scan, but no pagination.
It also prints a very annoying warning (printing, not emitting a warning,
so that warning filters are not applicable), to make sure that anyone
who picks it up and runs it verbatim gets some signal that they ought to
think about it and possibly make some changes.
The associated doc notes that you could make this method raise an error
and it would mostly work, but that some SDK features would fail if you
do so.

In order to make the DynamoDB token storage example pass mypy linting and
use the current paradigm for TokenStorage, it needs:

- missing annotations (easy)
- retrofitting of existing functions (easy)
- mypy overrides to ignore `boto3` types (easy, but note below)
- an implementation of `get_token_data_by_resource_server` (see note!)

For mypy, I have decided to move to a dedicated `mypy.ini` config in the
`docs/` directory. With the config growing, it didn't make sense to keep
it embedded in the repo root pyproject.toml . That also lets us set
different top-level behavioral flags in the config.

For `get_token_data_by_resource_server`, I decided to implement it, in
the simplest possible way using a sequential scan, but no pagination.
It also prints a very annoying warning (printing, not emitting a warning,
so that warning filters are not applicable), to make sure that anyone
who picks it up and runs it verbatim gets some signal that they ought to
think about it and possibly make some changes.
The associated doc notes that you _could_ make this method raise an error
and it would mostly work, but that some SDK features would fail if you
do so.
@sirosen
sirosen requested a review from aaschaer as a code owner August 14, 2026 18:29
@sirosen sirosen added the no-news-is-good-news This change does not require a news file label Aug 14, 2026
@sirosen sirosen added the no-news-is-good-news This change does not require a news file label Aug 14, 2026

@derek-globus derek-globus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My only real note is around the term Adapter.

The old interface was called StorageAdapater, the new one is called TokenStorage. In my opinion, child classes shouldn't be referred to as "adapaters" in class name or text references to avoid confusion between the two versions.

Comment thread docs/examples/token_storage/dynamodb_token_storage.py Outdated
Comment thread docs/examples/token_storage/index.rst Outdated
Comment on lines +12 to +13
Unlike the builtin adapters for JSON and sqlite, enumerating tokens in storage
is not a desirable operation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
Unlike the builtin adapters for JSON and sqlite, enumerating tokens in storage
is not a desirable operation.
Unlike the builtin ones, enumerating tokens in a DynamoDB table-backed storage
is not desirable.

Mostly a recommendation to avoid the word "adapter" here & in the rest of the document.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I made a slight alteration in this when I applied, but spiritually it's the same. LMK if you want it tweaked more. 👍

Avoid "Adapter", which echoes the legacy "StorageAdapter" interfaces.

Co-authored-by: derek-globus <113056046+derek-globus@users.noreply.github.com>
@sirosen
sirosen merged commit 4f26945 into globus:main Aug 14, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-news-is-good-news This change does not require a news file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants