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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: Add Retry Guide #10598
docs: Add Retry Guide #10598
Changes from 9 commits
c766790
787cc94
95e4731
e1b2b6b
9e6df27
06bdaf7
1ed75de
e7fc239
a36ba21
1ae4060
c18a88d
9f369fc
c0aed1e
6665e12
fd095c0
b179adc
73c81c6
4e50c50
e876c32
cc5ef28
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: consider putting this in backticks
server temporarily unavailable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. I think it should be clearer now that it's not a status code or message back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: consider rewording
Cloud Service
to something likeDefault retry settings are configured per RPC by each backend cloud service
. Probably me being paranoid, butService
is such an overloaded term, I don't want there to be confusion about a "new" proper nounThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, updated. I tried rewording it to make it clearer. I didn't want to introduce the term backend, but I think it should make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider linking
Retry Status Codes
to where you define them later (line 51) - I think you can do that using anchor tags (https://gist.github.com/rachelhyman/b1f109155c9dafffe618)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Let me try and move
Configurable Retry Params
section up.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"When jitter is enabled" implies that it can be disabled, but
setJittered
is now deprecated. Can we change the wording to indicate thatJitter is always enabled
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do, thanks!