{"action":"created","issue":{"url":"https://api.github.com/repos/stripe/stripe-python/issues/466","repository_url":"https://api.github.com/repos/stripe/stripe-python","labels_url":"https://api.github.com/repos/stripe/stripe-python/issues/466/labels{/name}","comments_url":"https://api.github.com/repos/stripe/stripe-python/issues/466/comments","events_url":"https://api.github.com/repos/stripe/stripe-python/issues/466/events","html_url":"https://github.com/stripe/stripe-python/pull/466","id":352302167,"node_id":"MDExOlB1bGxSZXF1ZXN0MjA5NjQ4MTAy","number":466,"title":"Implement retry mechanism when using requests","user":{"login":"jonathan-s","id":2124818,"node_id":"MDQ6VXNlcjIxMjQ4MTg=","avatar_url":"https://avatars3.githubusercontent.com/u/2124818?v=4","gravatar_id":"","url":"https://api.github.com/users/jonathan-s","html_url":"https://github.com/jonathan-s","followers_url":"https://api.github.com/users/jonathan-s/followers","following_url":"https://api.github.com/users/jonathan-s/following{/other_user}","gists_url":"https://api.github.com/users/jonathan-s/gists{/gist_id}","starred_url":"https://api.github.com/users/jonathan-s/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jonathan-s/subscriptions","organizations_url":"https://api.github.com/users/jonathan-s/orgs","repos_url":"https://api.github.com/users/jonathan-s/repos","events_url":"https://api.github.com/users/jonathan-s/events{/privacy}","received_events_url":"https://api.github.com/users/jonathan-s/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2018-08-20T21:25:00Z","updated_at":"2018-08-21T21:09:14Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/stripe/stripe-python/pulls/466","html_url":"https://github.com/stripe/stripe-python/pull/466","diff_url":"https://github.com/stripe/stripe-python/pull/466.diff","patch_url":"https://github.com/stripe/stripe-python/pull/466.patch"},"body":"This PR implements a retry mechanism if any request would fail when using requests. It closes #464. \r\n\r\nMore detailed documentation of `Retry` can be found in the [urllib3 documentation](https://urllib3.readthedocs.io/en/latest/reference/urllib3.util.html#urllib3.util.retry.Retry)\r\n\r\n* Total: The amount of total retries\r\n* backoff_factor: it will sleep for: `{backoff factor} * (2 ^ ({number of total retries} - 1))` between retries\r\n* status: How many times to retry on bad status codes. The ones on status_forcelist\r\n* status_forcelist: Bad status codes which it will retry on. \r\n\r\n"},"comment":{"url":"https://api.github.com/repos/stripe/stripe-python/issues/comments/414822558","html_url":"https://github.com/stripe/stripe-python/pull/466#issuecomment-414822558","issue_url":"https://api.github.com/repos/stripe/stripe-python/issues/466","id":414822558,"node_id":"MDEyOklzc3VlQ29tbWVudDQxNDgyMjU1OA==","user":{"login":"mickjermsurawong-stripe","id":42550941,"node_id":"MDQ6VXNlcjQyNTUwOTQx","avatar_url":"https://avatars0.githubusercontent.com/u/42550941?v=4","gravatar_id":"","url":"https://api.github.com/users/mickjermsurawong-stripe","html_url":"https://github.com/mickjermsurawong-stripe","followers_url":"https://api.github.com/users/mickjermsurawong-stripe/followers","following_url":"https://api.github.com/users/mickjermsurawong-stripe/following{/other_user}","gists_url":"https://api.github.com/users/mickjermsurawong-stripe/gists{/gist_id}","starred_url":"https://api.github.com/users/mickjermsurawong-stripe/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mickjermsurawong-stripe/subscriptions","organizations_url":"https://api.github.com/users/mickjermsurawong-stripe/orgs","repos_url":"https://api.github.com/users/mickjermsurawong-stripe/repos","events_url":"https://api.github.com/users/mickjermsurawong-stripe/events{/privacy}","received_events_url":"https://api.github.com/users/mickjermsurawong-stripe/received_events","type":"User","site_admin":false},"created_at":"2018-08-21T21:09:14Z","updated_at":"2018-08-21T21:09:14Z","author_association":"NONE","body":"Hi Jonathan! Thank you for your time and this PR :)\r\n\r\nTo protect against creating unintended duplicate requests given automatic retries, the client should create idempotency key, if not given by the caller, following our docs[0].\r\n\r\nThe implementation on retry seems great with the standard `Retry`. It has flexibility to handle the connection/read failures in `connect=Integer`, `read=Integer` and special error status code `status_forcelist=[]` similar to behavior of our implementations in other languages. However, it lacks one detail on adding randomness to the delay to avoid simultaneously backing-off across multiple clients (step-wise jamming), following our implementations in Ruby[1] or Go[2]. \r\n\r\nMy concern of going ahead with using `Retry` specific to `requests` client is that it can't support our other implementations in `pycurl`, `urlfetch`, `urllib.request`. Because the retry is tied in `request` execution method, we might have to implement an equivalent for each of other client at the request level. Ideally, we can move this retry logic up at the parent class. Concretely, we can possibly have `request_with_retry` at `HTTPClient` parent class, surrounding `request` method implemented by each library with our custom retry logic. (We still have to implement retry condition for exceptions thrown from each kind of client)\r\n\r\n-cc @brandur-stripe for more thoughts if we should go ahead with `Retry` given that `request`'s popularity, but we incur more implementation work in other libs (if we want to support retry there). \r\n\r\n\r\n[0] Idempotency\r\nhttps://stripe.com/docs/api/ruby#idempotent_requests\r\n\r\n[1] Ruby sleep implementation\r\nhttps://github.com/stripe/stripe-ruby/blob/ec66c3f0f44274f885de8d13de5dce2657932121/lib/stripe/stripe_client.rb#L80\r\n\r\n[2] Go sleep implementation\r\nhttps://github.com/stripe/stripe-go/blob/442690c3856640e4913163f506f0070561f3368d/stripe.go#L511\r\n"}}
{"actor":{"display_login":"mickjermsurawong-stripe"}}