Bugfix/devexp 765 syncronize token managers#156
Conversation
marcos-sinch
requested review from
Hurus111,
JPPortier,
asein-sinch,
edu-sinch and
matsk-sinch
as code owners
|
Hi @marcos-sinch : target branch should be 2.1 next but not main |
|
| f"Sync HTTP response {response.status_code} with headers: {response.headers}" | ||
| f" and body: {response_body} from URL: {request_data.url}" |
There was a problem hiding this comment.
Not related to this PR but curious about python and "overload" here.
A logger is used
- which will decide if message is having to be send regarding severity of message and config level for logger
- finally message won't be send by logger because of severity is configured to "warn" (debug is "lower")
- but here, sent of not by logger, the message is formatted before being sent to logger
Right ?
This mean with "heavy" calls to send: "heavy" load due to formatting which won't be consumed.
Can we do "more efficient" ?
- format message from logger if having to be sent by accepting list of parameter to be used when formatting message (
logger.debug("Processing %s with data %r", value1, value2)) - guard before call:
if logger.isEnabledFor(logging.DEBUG): - provide a function to be called by logger if it decide to send the message (like Supplier in Java? )
- ???
| HTTP call, deserialize the response, and return an ``HTTPResponse``. | ||
| They should **not** handle token refresh — that is done by | ||
| ``request``. | ||
| def send(self, request_data: HttpRequest) -> HTTPResponse: |
There was a problem hiding this comment.
This will be a breaking change for any existing integration.
We should find another way to address the migration
| response=None | ||
| ) | ||
|
|
||
| def _fetch_new_token(self) -> OAuthToken: |
There was a problem hiding this comment.
Function name may be not ideal ?
It do not only fetch a new token, it also store it in instance itself (then duplicate code from set_auth_token)
| Thread-safe synchronous OAuth token manager. | ||
| """ | ||
|
|
||
| def get_auth_token(self) -> OAuthToken: |
There was a problem hiding this comment.
Same comment here about naming: it do not get the token in term of "getter": it also refresh and store it
2 participants
Synchronize token managers and refactor HTTP transport
Fixes a race condition in OAuth token creation and renewal under concurrent requests.
Token manager (
token_manager.py)threading.LocktoTokenManagerBase.get_auth_tokenuses double-checked locking so that the initial token fetch happens exactly once even when many threads call it simultaneously.EXPIREDstate +handle_invalid_tokenside-channel with a newrefresh_auth_token(used_token)method: whichever thread holds the lock compares the used (stale) token against the currently cached one and fetches a new token only if they still match, deduplicating concurrent renewal attempts.TokenState.EXPIREDvalue.HTTP transport (
http_transport.py,requests_http_transport.py)prepare_requestandauthenticateout ofsend()and intorequest(), sosend()now receives an already-preparedHttpRequestinstead of a rawHTTPEndpoint. This makessend()a pure I/O primitive and simplifies subclassing._get_bearer_token_from_requestand_set_bearer_tokenhelpers to keep the token-stamping logic in one place.Tests
MockHTTPTransporttest harness with tests that drive the realHTTPTransportRequestsadapter, giving better coverage of the actual I/O path.get_auth_tokenandrefresh_auth_tokenthat assert a single fetch under 20 simultaneous threads.