-
Notifications
You must be signed in to change notification settings - Fork 38
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
feat(v2): add OnHTTPCodes CallOption #188
Conversation
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.
LGTM overall, just a couple of small things.
} | ||
|
||
func (r *httpRetryer) Retry(err error) (time.Duration, bool) { | ||
var gerr *googleapi.Error |
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.
Side note: Maybe we should do the work to wrap these with APIError so gax only opperates on this type directly...
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'm not sure what you mean, can you elaborate? The main sticking point for me is that APIError
doesn't have a Code
property (we'd need to decide on a transport-agnostic Code
type or align on gRPC Codes).
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.
Sorry, I mean in apiary-land we should consider starting to wrap those errors with APIError. I believe APIError has access to the raw googleapis.Error. Might make a PR to wrap things later...
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.
Thanks!
This adds the
OnHTTPCodes
CallOption
for configuring aRetryer
to retry a set of HTTP status codes present in anerror
of typegoogleapi.Error
.