Repository navigation
OAuth cache improvements - #435
Conversation
5632003 to
292bd0f
Compare
|
|
||
| @staticmethod | ||
| def _construct_cache_key(host: Optional[str], user: Optional[str]) -> str: | ||
| return f"{host}@{user}" |
There was a problem hiding this comment.
usually people expect stuff to be user@host but it doesn't matter here since it's an internal cache key (unless we log it somewhere)
hashhar
left a comment
hashhar
left a comment
There was a problem hiding this comment.
first commit is unclear to me, can you explain a bit about it?
2nd looks good.
|
If there is no backend available then keyring backend is being initialized as |
|
|
||
| @staticmethod | ||
| def _determine_user(headers: Mapping[Any, Any]) -> Optional[Any]: | ||
| return headers.get(HEADER_USER) |
There was a problem hiding this comment.
X-Trino-User header is optional
There was a problem hiding this comment.
You're right, is there any other way to get username?
There was a problem hiding this comment.
Since the actual user might come from user mapping then getting it from the server is probably the only option.
There was a problem hiding this comment.
maybe we can use the client provided principal/user instead of user which it gets resolved to on the server to bypass this problem entirely?
292bd0f to
8c4c75f
Compare
hashhar
left a comment
hashhar
left a comment
There was a problem hiding this comment.
please update docs to add warning what happens when you don't specify user when connecting (token cache gets shared)
|
|
||
| def get_token_from_cache(self, host: Optional[str]) -> Optional[str]: | ||
| return self._cache.get(host) | ||
| def get_token_from_cache(self, key: Optional[str]) -> Optional[str]: |
There was a problem hiding this comment.
if you extract the host -> key rename to it's own commit functional changes will be more visible and easier to see.
8c4c75f to
9064581
Compare
hashhar
left a comment
hashhar
left a comment
There was a problem hiding this comment.
LGTM.
@lukasz-walkiewicz do you want to take a look?
We now do like JDBC driver, if username was specified we use host+user cache key otherwise we use only host as the cache key and tokens get shared (same as JDBC driver).
3 participants
Description
Closes #430 and #431
Non-technical explanation
Release notes
( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: