Skip to content
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

Allow setting stripe.max_network_retries to affect HTTPClients #481

Conversation

timvisher
Copy link
Contributor

Motivation

Before this patch, setting stripe.max_network_retries had no affect on
the following code.

import stripe
import sys

stripe.api_key = sys.argv[1]
stripe.max_network_retries = 5
client = stripe.http_client.RequestsClient(timeout=0.1)
stripe.default_http_client = client
stripe.log = 'info'

while True:
    try:
        charges = stripe.Charge.list()
    except stripe.error.APIConnectionError as e:
        print(e.user_message)
        break
message='Request to Stripe api' method=get path=https://api.stripe.com/v1/charges
Unexpected error communicating with Stripe.  If this problem persists,
let us know at support@stripe.com.

(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))

After the patch, I can utilize stripe.max_network_retries effectively.

message='Request to Stripe api' method=get path=https://api.stripe.com/v1/charges
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 1 for request get https://api.stripe.com/v1/charges after sleeping 0.50 seconds.'
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 2 for request get https://api.stripe.com/v1/charges after sleeping 0.98 seconds.'
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 3 for request get https://api.stripe.com/v1/charges after sleeping 1.87 seconds.'
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 4 for request get https://api.stripe.com/v1/charges after sleeping 1.99 seconds.'
Unexpected error communicating with Stripe.  If this problem persists,
let us know at support@stripe.com.

(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))

Cross Reference

Workaround

Until this is merged it's possible to work around this issue by directly
setting the HTTPClient's _max_network_retries method:

import stripe
import sys

stripe.api_key = sys.argv[1]
client = stripe.http_client.RequestsClient(timeout=0.1)
client._max_network_retries = lambda: 5
stripe.default_http_client = client
stripe.log = 'info'

while True:
    try:
        charges = stripe.Charge.list()
    except stripe.error.APIConnectionError as e:
        print(e.user_message)
        break
message='Request to Stripe api' method=get path=https://api.stripe.com/v1/charges
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 1 for request get https://api.stripe.com/v1/charges after sleeping 0.50 seconds.'
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 2 for request get https://api.stripe.com/v1/charges after sleeping 0.59 seconds.'
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 3 for request get https://api.stripe.com/v1/charges after sleeping 1.77 seconds.'
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 4 for request get https://api.stripe.com/v1/charges after sleeping 1.98 seconds.'
Unexpected error communicating with Stripe.  If this problem persists,
let us know at support@stripe.com.

(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))~

@timvisher
Copy link
Contributor Author

Happy to squash after approval. :)

@brandur-stripe
Copy link
Contributor

@timvisher Thanks for the contribution Tim! I don't think I 100% follow the patch — do you know why the variable must be explicitly imported under certain conditions?

@timvisher
Copy link
Contributor Author

@brandur-stripe IIUC, it's not that it's under certain conditions but that python doesn't propagate values changes to module variables after import.

I don't have an easy way to demonstrate this in a shell, I don't think, but basically once I have value imported, if I set it again the module that imported it doesn't see the update. The way to get the update is to re-import it.

I may be wrong on this point though. I'm more of a Clojure rather than a Python dev. :)

@timvisher
Copy link
Contributor Author

I assume, given the Cross Reference, that this is either a common pattern or the only way to accomplish this while still maintaining the module API constraints (setting things on stripe.* rather than initializing sub-components with some new value).

@brandur-stripe
Copy link
Contributor

Ah, yes that makes sense. Would you mind squashing then? This looks good to me.

Motivation
----------

Before this patch, setting `stripe.max_network_retries` had no affect on
the following code.

```
import stripe
import sys

stripe.api_key = sys.argv[1]
stripe.max_network_retries = 5
client = stripe.http_client.RequestsClient(timeout=0.1)
stripe.default_http_client = client
stripe.log = 'info'

while True:
    try:
        charges = stripe.Charge.list()
    except stripe.error.APIConnectionError as e:
        print(e.user_message)
        break
```

```
message='Request to Stripe api' method=get path=https://api.stripe.com/v1/charges
Unexpected error communicating with Stripe.  If this problem persists,
let us know at support@stripe.com.

(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))
```

After the patch, I can utilize `stripe.max_network_retries` effectively.

```
message='Request to Stripe api' method=get path=https://api.stripe.com/v1/charges
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 1 for request get https://api.stripe.com/v1/charges after sleeping 0.50 seconds.'
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 2 for request get https://api.stripe.com/v1/charges after sleeping 0.98 seconds.'
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 3 for request get https://api.stripe.com/v1/charges after sleeping 1.87 seconds.'
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 4 for request get https://api.stripe.com/v1/charges after sleeping 1.99 seconds.'
Unexpected error communicating with Stripe.  If this problem persists,
let us know at support@stripe.com.

(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))
```

[Cross Reference][cross]

[cross]: https://github.com/stripe/stripe-python/blob/ae953fd0aa531f5b500e5e86eee5859df95a255d/stripe/api_requestor.py#L239

Workaround
----------

Until this is merged it's possible to work around this issue by directly
setting the HTTPClient's `_max_network_retries` method:

```
import stripe
import sys

stripe.api_key = sys.argv[1]
client = stripe.http_client.RequestsClient(timeout=0.1)
client._max_network_retries = lambda: 5
stripe.default_http_client = client
stripe.log = 'info'

while True:
    try:
        charges = stripe.Charge.list()
    except stripe.error.APIConnectionError as e:
        print(e.user_message)
        break
```

```
message='Request to Stripe api' method=get path=https://api.stripe.com/v1/charges
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 1 for request get https://api.stripe.com/v1/charges after sleeping 0.50 seconds.'
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 2 for request get https://api.stripe.com/v1/charges after sleeping 0.59 seconds.'
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 3 for request get https://api.stripe.com/v1/charges after sleeping 1.77 seconds.'
message="Encountered a retryable error Unexpected error communicating with Stripe.  If this problem persists,\nlet us know at support@stripe.com.\n\n(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))"
message='Initiating retry 4 for request get https://api.stripe.com/v1/charges after sleeping 1.98 seconds.'
Unexpected error communicating with Stripe.  If this problem persists,
let us know at support@stripe.com.

(Network error: ReadTimeout: HTTPSConnectionPool(host='api.stripe.com', port=443): Read timed out. (read timeout=0.1))~
```
@timvisher timvisher force-pushed the fix/allow-setting-max_network-retries branch from 1065297 to 761105a Compare October 2, 2018 19:46
@timvisher
Copy link
Contributor Author

All done (in case you didn't see the squash). :)

@brandur-stripe
Copy link
Contributor

Thanks again!

@brandur-stripe brandur-stripe merged commit 5c02b88 into stripe:master Oct 2, 2018
@brandur-stripe
Copy link
Contributor

Released as 2.10.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants