Support object as value in extra_credential #462
Conversation
huw0
force-pushed
the
support-extra-credential-object
branch
3 times, most recently
from
15dc44c
to
47acd94
Compare
tests/unit/test_client.py
Outdated
| port=constants.DEFAULT_TLS_PORT, | ||
| client_session=ClientSession( | ||
| user="test", | ||
| extra_credential=[("foo", TestCredential())] |
There was a problem hiding this comment.
I get why you made this change, makes it easier to use object as cred values. But how is this different from below? Just want to understand what problem it actually solves.
extra_credential=[("foo"), str(TestCredential())]
i.e. how do other users of this API learn that the object's __str__ implementation is actually meaningful and is what will be transported.
There was a problem hiding this comment.
If TestCredential() provides a time limited credential (JWT or otherwise) then the object can refresh the underlying credential. This has two benefits:
- No need to regenerate TrinoClient object to refresh the credential.
- If the time-limited credential expiry is hit, the automatic retry should include the refreshed credential in the next API call.
There was a problem hiding this comment.
Somehow I missed that your comment was on the test! I've added a second assertion which hopefully is both a more thorough test and shows the difference clearly.
huw0
force-pushed
the
support-extra-credential-object
branch
7 times, most recently
from
c540f50
to
a309986
Compare
huw0
force-pushed
the
support-extra-credential-object
branch
from
a309986
to
37680dd
Compare
|
I've rebased the branch and improved the unit test. |
|
Sorry for the delay here. Thanks for the contribution. |
Description
I want
extra_credentialto accept an object as a value. This is helpful because it allows pass through of short lived tokens (JWT or otherwise) that can be refreshed by the client without having to generate a new engine/client object.Presently this is not possible because
parse_quotedoes a check on the object type.Therefore it is necessary to call
str()on theextra_credentialvalue during serialisation.Non-technical explanation
Support object as value in
extra_credentialto support refreshing tokens.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.
(x) Release notes are required, with the following suggested text: