feat(sdk): Support refreshing the access token in Client::fetch_server_versions#5916
Merged
Merged
Conversation
Allows to have stricter visibility for the fields and put less code in ClientBuilder. Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5916 +/- ##
==========================================
- Coverage 88.66% 88.65% -0.01%
==========================================
Files 363 363
Lines 104579 104690 +111
Branches 104579 104690 +111
==========================================
+ Hits 92726 92818 +92
- Misses 7505 7518 +13
- Partials 4348 4354 +6 ☔ View full report in Codecov by Sentry. |
This will allow to handle automatically whether to send an access token or not on endpoints that don't require it in contexts were can't refresh it. We also don't cache calls to GET /versions that were not authenticated, because they might lack some features compared to an authenticated request. Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
We need to handle 2 possible deadlocks for this: 1. We cannot try to refresh an expired access token if this call happens while we are currently trying to refresh the token. The easiest way to handle this is to never try to refresh the token when making this call inside `get_path_builder_input()` so we implement a "failsafe" mode that disables refreshing the access token in case it expired. However it attempts the GET /versions again without the token. 2. We cannot access the cached supported versions if we are in the process of refreshing that cache because the RwLock has a write lock. So if the access token has expired and we try to refresh it, the possible calls to `get_path_builder_input()` must not wait for a read lock to be available. So the solution is to never wait for a read lock, and skip the cache if a read lock is not available. This also gets rid of workarounds in other functions. Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
e031e61 to
564e652
Compare
CodSpeed Performance ReportMerging #5916 will not alter performanceComparing Summary
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We need to handle 2 possible deadlocks for this:
while we are currently trying to refresh the token. The easiest way
to handle this is to never try to refresh the token when making this
call inside
get_path_builder_input()so we implement a "failsafe"mode that disables refreshing the access token in case it expired.
However it attempts the GET /versions request again without the token.
process of refreshing that cache because the RwLock has a write lock.
So if the access token has expired and we try to refresh it, the
possible calls to
get_path_builder_input()must not wait for a readlock to be available. So the solution is to never wait for a read
lock, and skip the cache if a read lock is not available.
This also gets rid of workarounds in other functions.