Support custom cache for OAuth2 tokens #225
Conversation
trino/auth.py
Outdated
| if custom_cache is not None: | ||
| return custom_cache |
There was a problem hiding this comment.
Maybe is worth to add some validation and error handling for custom_cache?
There was a problem hiding this comment.
I've added an isinstance check. I don't think there is anything more we can do. We might rethrow the exception as one of the dbapi exception types. WDYT?
README.md
Outdated
| @@ -198,6 +198,8 @@ A callback to handle the redirect url can be provided via param `redirect_auth_u | |||
|
|
|||
| The OAuth2 token will be cached either per `trino.auth.OAuth2Authentication` instance or, when keyring is installed, it will be cached within a secure backend (MacOS keychain, Windows credential locker, etc) under a key including host of the Trino connection. Keyring can be installed using `pip install 'trino[external-authentication-token-cache]'`. | |||
|
|
|||
| A custom caching implementation can be provided by creating a class implementing the `trino.auth.OAuth2TokenCache` abstract class and adding it as in `OAuth2Authentication(cache=my_custom_cache_impl)`. The custom caching implementation enables usage in multi-user environments (notebooks, web applications) in combination with a custom `redirect_auth_url_handler` as explained above. | |||
There was a problem hiding this comment.
Let's explain further what can be passed as my_custom_cache_impl or give some code snippets here.
mdesmet
force-pushed
the
feature/custom-cache
branch
2 times, most recently
from
c04d408 to
709d718
Compare
mdesmet
force-pushed
the
feature/custom-cache
branch
from
709d718 to
42ccb60
Compare
|
I'd prefer if we made the defaults more useful instead of adding hooks for custom behaviour and leaving users to implement them themselves. It seems default should be to cache based on identity (which is what Trino itself seems to do). |
That won't work in a distributed web application. You would need to store and retrieve the tokens in a distributed cache (eg redis) as every request may be handled by different nodes.
This is not entirely true. The user is not required in the |
Fixes #223