-
Notifications
You must be signed in to change notification settings - Fork 436
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
Python retry mechanism #470
Python retry mechanism #470
Conversation
cc @jonathan-s |
cc @stripe/api-libraries |
http://docs.python-requests.org/en/master/_modules/requests/exceptions/ |
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.
Looks great @mickjermsurawong-stripe! Thanks for working on this.
Alright, this looks great to me, but the feature is complex enough that I'd like to get another set of eyes on it. |
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.
Left a couple minor comments, but thanks for being so thorough on this one! Looks excellent.
Added comment/logging |
LGTM! Thanks. |
self-approve after lgtm ^ |
Released in 2.8 fb8117d |
Ended up needing this feature and came across this thread to check out the implementation. Wanted to drop a belated "thank you" for this PR! |
Summary:
requests
http-client, but not others due to overwhelming majority of our clients. This design allows future extension the support to other http-clients. (As opposed to urllib3Retry
that is coupled at the execution invocation.Concerns:
requests
module from within test, and that results in mocking kinds of exception classes that we want to handle (existing tests also rely on the similar mechanism)Motivation:
[0] Ruby retry conditions
https://github.com/stripe/stripe-ruby/blob/ec66c3f0f44274f885de8d13de5dce2657932121/lib/stripe/stripe_client.rb#L61
[1] Go retry conditions
https://github.com/stripe/stripe-go/blob/442690c3856640e4913163f506f0070561f3368d/stripe.go#L495
[2] Ruby sleep
https://github.com/stripe/stripe-ruby/blob/ec66c3f0f44274f885de8d13de5dce2657932121/lib/stripe/stripe_client.rb#L80
[3] Go sleep
https://github.com/stripe/stripe-go/blob/442690c3856640e4913163f506f0070561f3368d/stripe.go#L511