feat: Retry behavior #1113
Merged
feat: Retry behavior #1113
Conversation
This file contains 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
clundin25
force-pushed
the
property-retry
branch
2 times, most recently
from
3f634f6
to
3593bab
Compare
clundin25
added
kokoro:force-run
gcf-owl-bot
bot
removed
the
owlbot:run
yoshi-kokoro
removed
kokoro:run
clundin25
force-pushed
the
property-retry
branch
from
3593bab
to
185349c
Compare
clundin25
added
kokoro:force-run
clundin25
force-pushed
the
property-retry
branch
from
185349c
to
9229690
Compare
yoshi-kokoro
removed
kokoro:run
clundin25
added
kokoro:force-run
yoshi-kokoro
removed
kokoro:run
clundin25
force-pushed
the
property-retry
branch
from
9229690
to
100dd2f
Compare
clundin25
added
kokoro:force-run
yoshi-kokoro
removed
kokoro:run
clundin25
force-pushed
the
property-retry
branch
from
100dd2f
to
b2c3315
Compare
clundin25
added
kokoro:force-run
yoshi-kokoro
removed
kokoro:run
clundin25
force-pushed
the
property-retry
branch
from
b2c3315
to
80d90ed
Compare
yoshi-kokoro
removed
the
kokoro:force-run
clundin25
added
kokoro:force-run
yoshi-kokoro
removed
kokoro:run
gcf-owl-bot
bot
removed
the
owlbot:run
|
@parthea do you mind reviewing this for Python readability? |
google/oauth2/_client_async.py
Outdated
|
|
||
| try: | ||
| access_token = response_data["access_token"] | ||
| except KeyError as caught_exc: | ||
| new_exc = exceptions.RefreshError("No access token in response.", response_data) | ||
| new_exc = exceptions.RefreshError( | ||
| "No access token in response.", response_data, retryable=True |
There was a problem hiding this comment.
why this is by default Retryable?
There was a problem hiding this comment.
if it is expected why we expect the retry to succeed?
There was a problem hiding this comment.
My interpretation was that should this happen we have received a malformed request or error, that ideally should have been handled at the transport layer.
I think the best path is to retry it to see if that resolves the issue.
If you disagree I am happy to set this to False. It is hard to decide what is right :)
There was a problem hiding this comment.
Let's have them as false since that is the original behavior
We don't want to retry those because - it is not a valid case, we don't expect it often enough to retry just in case
google/oauth2/_client_async.py
Outdated
|
|
||
| try: | ||
| id_token = response_data["id_token"] | ||
| except KeyError as caught_exc: | ||
| new_exc = exceptions.RefreshError("No ID token in response.", response_data) | ||
| new_exc = exceptions.RefreshError( | ||
| "No ID token in response.", response_data, retryable=True |
There was a problem hiding this comment.
same question on a default retryable
There was a problem hiding this comment.
like above, lets switch to false
clundin25
added
kokoro:force-run
gcf-owl-bot
bot
removed
the
owlbot:run
yoshi-kokoro
removed
kokoro:run
BigTailWolf
pushed a commit
to BigTailWolf/google-auth-library-python
that referenced
this pull request
Sep 22, 2022
* feat: Retry behavior * Introduce `retryable` property to auth library exceptions. This can be used to determine if an exception should be retried. * Introduce `should_retry` parameter to token endpoints. If set to `False` the auth library will not retry failed requests. If set to `True` the auth library will retry failed requests. The default value is `True` to maintain existing behavior. * Expanded list of HTTP Status codes that will be retried. * Modified retry behavior to use exponential backoff. * Increased default retry attempts from 2 to 3.
BigTailWolf
added a commit
to BigTailWolf/google-auth-library-python
that referenced
this pull request
Sep 22, 2022
This reverts commit 06fff40.
This was referenced Nov 7, 2022
Closed
This was referenced Sep 1, 2023
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.
retryableproperty to auth library exceptions. This can beused to determine if an exception should be retried.
should_retryparameter to token endpoints. If set toFalsethe auth library will not retry failed requests. If set to
Truetheauth library will retry failed requests. The default value is
Trueto maintain existing behavior.