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

Tdl 17878 implement request timeout and retry #126

Merged
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ Create a config file containing the stripe credentials, e.g.:
{
"client_secret": "sk_live_xxxxxxxxxxxxxxxxxxxxxxxx",
"account_id": "acct_xxxxxxxxxxxxxxxx",
"start_date": "2017-01-01T00:00:00Z"
"start_date": "2017-01-01T00:00:00Z",
"request_timeout": 300
}
```

Expand Down
15 changes: 12 additions & 3 deletions tap_stripe/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@

DEFAULT_DATE_WINDOW_SIZE = 30 #days

# default request timeout
REQUEST_TIMEOUT = 300 # 5 minutes
class Context():
config = {}
state = {}
Expand Down Expand Up @@ -186,11 +188,18 @@ def configure_stripe_client():
stripe.api_key = Context.config.get('client_secret')
# Override the Stripe API Version for consistent access
stripe.api_version = '2020-08-27'
# Allow ourselves to retry retriable network errors 5 times
# Allow ourselves to retry retryable network errors 15 times
# https://github.com/stripe/stripe-python/tree/a9a8d754b73ad47bdece6ac4b4850822fa19db4e#configuring-automatic-retries
stripe.max_network_retries = 15
# Configure client with a default timeout of 80 seconds
client = stripe.http_client.RequestsClient()

request_timeout = Context.config.get('request_timeout')
# if request_timeout is other than 0, "0" or "" then use request_timeout
if request_timeout and float(request_timeout):
request_timeout = float(request_timeout)
else: # If value is 0, "0" or "" then set default to 300 seconds.
request_timeout = REQUEST_TIMEOUT
# configure the clint with the request_timeout
client = stripe.http_client.RequestsClient(timeout=request_timeout)
apply_request_timer_to_client(client)
stripe.default_http_client = client
# Set stripe logging to INFO level
Expand Down
72 changes: 72 additions & 0 deletions tests/unittests/test_request_timeout.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import unittest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have test cases of retry?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We haven't implemented the retry logic. It is handled in the SDK itself, and the max_network_retries was already present in the code. Hence we have not written testcases for retry.

from unittest import mock
from tap_stripe import Context, configure_stripe_client

class TestRequestTimeoutValue(unittest.TestCase):
'''
Test that request timeout parameter works properly in various cases
'''
@mock.patch('stripe.http_client.RequestsClient')
@mock.patch('tap_stripe.apply_request_timer_to_client')
@mock.patch('stripe.Account.retrieve')
def test_config_provided_request_timeout(self, mock_retrieve, mock_req_timer, mock_client):
"""
Unit tests to ensure that request timeout is set based on config value
"""
config = { "client_secret": "test_secret", "account_id": "test_account", "start_date": "test_start_date", "request_timeout": 100}
Context.config = config
configure_stripe_client()
# Verify that the client is called with config provided request timeout
mock_client.assert_called_with(timeout=100.0)

@mock.patch('stripe.http_client.RequestsClient')
@mock.patch('tap_stripe.apply_request_timer_to_client')
@mock.patch('stripe.Account.retrieve')
def test_default_value_request_timeout(self, mock_retrieve, mock_req_timer, mock_client):
"""
Unit tests to ensure that request timeout is set based default value
"""
config = {"client_secret": "test_secret", "account_id": "test_account", "start_date": "test_start_date"}
Context.config = config
configure_stripe_client()
# Verify that the client is called with default request timeout
mock_client.assert_called_with(timeout=300.0)

@mock.patch('stripe.http_client.RequestsClient')
@mock.patch('tap_stripe.apply_request_timer_to_client')
@mock.patch('stripe.Account.retrieve')
def test_config_provided_empty_request_timeout(self, mock_retrieve, mock_req_timer, mock_client):
"""
Unit tests to ensure that request timeout is set based on default value if empty value is given in config
"""
config = {"client_secret": "test_secret", "account_id": "test_account", "request_timeout": ""}
Context.config = config
configure_stripe_client()
# Verify that the client is called with default request timeout
mock_client.assert_called_with(timeout=300.0)

@mock.patch('stripe.http_client.RequestsClient')
@mock.patch('tap_stripe.apply_request_timer_to_client')
@mock.patch('stripe.Account.retrieve')
def test_config_provided_string_request_timeout(self, mock_retrieve, mock_req_timer, mock_client):
"""
Unit tests to ensure that request timeout is set based on config string value
"""
config = {"client_secret": "test_secret", "account_id": "test_account", "request_timeout": "100"}
Context.config = config
configure_stripe_client()
# Verify that the client is called with config provided request timeout
mock_client.assert_called_with(timeout=100.0)

@mock.patch('stripe.http_client.RequestsClient')
@mock.patch('tap_stripe.apply_request_timer_to_client')
@mock.patch('stripe.Account.retrieve')
def test_config_provided_float_request_timeout(self, mock_retrieve, mock_req_timer, mock_client):
"""
Unit tests to ensure that request timeout is set based on config float value
"""
config = {"client_secret": "test_secret", "account_id": "test_account", "request_timeout": 100.8}
Context.config = config
configure_stripe_client()
# Verify that the client is called with config provided float request timeout
mock_client.assert_called_with(timeout=100.8)