diff --git a/.circleci/config.yml b/.circleci/config.yml index ff0c01d..d34c99d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -26,7 +26,9 @@ jobs: name: 'Unit Tests' command: | source /usr/local/share/virtualenvs/tap-xero/bin/activate - nosetests tests/unittests/ + pip install nose coverage + nosetests --with-coverage --cover-erase --cover-package=tap_xero --cover-html-dir=htmlcov tests/unittests + coverage html - run: name: 'Integration Tests' command: | diff --git a/tap_xero/client.py b/tap_xero/client.py index 17386d5..e89842a 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -12,11 +12,13 @@ import pytz import backoff import singer +from requests.exceptions import Timeout LOGGER = singer.get_logger() BASE_URL = "https://api.xero.com/api.xro/2.0" +REQUEST_TIMEOUT = 300 class XeroError(Exception): def __init__(self, message=None, response=None): @@ -183,7 +185,18 @@ def __init__(self, config): self.user_agent = config.get("user_agent") self.tenant_id = None self.access_token = None - + request_timeout = 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 + self.request_timeout = request_timeout + + # Backoff for 1 minute when the Timeout error occurs as the request will again backoff + # when timeout occurs in `check_platform_access()`, hence instead of setting max_tries as 5 + # setting the max_time of 60 seconds + @backoff.on_exception(backoff.constant, Timeout, max_time=60, interval=10, jitter=None) # Interval value not consistent if jitter not None def refresh_credentials(self, config, config_path): header_token = b64encode((config["client_id"] + ":" + config["client_secret"]).encode('utf-8')) @@ -197,7 +210,7 @@ def refresh_credentials(self, config, config_path): "grant_type": "refresh_token", "refresh_token": config["refresh_token"], } - resp = self.session.post("https://identity.xero.com/connect/token", headers=headers, data=post_body) + resp = self.session.post("https://identity.xero.com/connect/token", headers=headers, data=post_body, timeout=self.request_timeout) if resp.status_code != 200: raise_for_error(resp) @@ -210,7 +223,10 @@ def refresh_credentials(self, config, config_path): self.access_token = resp["access_token"] self.tenant_id = config['tenant_id'] - + # Backoff for 1 minute when the Timeout error occurs as the request will again backoff + # when timeout occurs in `refresh_credentials()`, hence instead of setting max_tries as 5 + # setting the max_time of 60 seconds + @backoff.on_exception(backoff.constant, Timeout, max_time=60, interval=10, jitter=None) # Interval value not consistent if jitter not None @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroInternalError), max_tries=3) @backoff.on_exception(retry_after_wait_gen, XeroTooManyInMinuteError, giveup=is_not_status_code_fn([429]), jitter=None, max_tries=3) def check_platform_access(self, config, config_path): @@ -227,12 +243,14 @@ def check_platform_access(self, config, config_path): # Validating the authorization of the provided configuration currencies_url = join(BASE_URL, "Currencies") request = requests.Request("GET", currencies_url, headers=headers) - response = self.session.send(request.prepare()) + response = self.session.send(request.prepare(), timeout=self.request_timeout) if response.status_code != 200: raise_for_error(response) + # Backoff till 60 seconds in case of Timeout error to keep it consistent with other backoffs + @backoff.on_exception(backoff.constant, Timeout, max_time=60, interval=10, jitter=None) # Interval value not consistent if jitter not None @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroInternalError), max_tries=3) @backoff.on_exception(retry_after_wait_gen, XeroTooManyInMinuteError, giveup=is_not_status_code_fn([429]), jitter=None, max_tries=3) def filter(self, tap_stream_id, since=None, **params): @@ -247,7 +265,7 @@ def filter(self, tap_stream_id, since=None, **params): headers["If-Modified-Since"] = since request = requests.Request("GET", url, headers=headers, params=params) - response = self.session.send(request.prepare()) + response = self.session.send(request.prepare(), timeout=self.request_timeout) if response.status_code != 200: raise_for_error(response) diff --git a/tests/unittests/test_request_timeout.py b/tests/unittests/test_request_timeout.py new file mode 100644 index 0000000..2016532 --- /dev/null +++ b/tests/unittests/test_request_timeout.py @@ -0,0 +1,192 @@ +from tap_xero.client import XeroClient +import unittest +from unittest import mock +from requests.exceptions import Timeout, ConnectTimeout +import datetime + +class TestBackoffError(unittest.TestCase): + ''' + Test that backoff logic works properly. + ''' + @mock.patch('tap_xero.client.requests.Request') + @mock.patch('tap_xero.client.requests.Session.send') + @mock.patch('tap_xero.client.requests.Session.post') + def test_backoff_check_platform_access_timeout_error(self, mock_post, mock_send, mock_request): + """ + Check whether the request backoffs properly for 120 seconds in case of Timeout error. + """ + mock_send.side_effect = Timeout + mock_post.side_effect = Timeout + before_time = datetime.datetime.now() + config = {"start_date": "dummy_st", "client_id": "dummy_ci", "client_secret": "dummy_cs", "tenant_id": "dummy_ti", "refresh_token": "dummy_rt"} + client = XeroClient(config) + client.access_token = "dummy_token" + with self.assertRaises(Timeout): + client.check_platform_access(config, "dummy_path") + after_time = datetime.datetime.now() + time_difference = (after_time - before_time).total_seconds() + self.assertGreaterEqual(time_difference, 120) + + @mock.patch('tap_xero.client.requests.Request') + @mock.patch('tap_xero.client.requests.Session.send') + @mock.patch('tap_xero.client.requests.Session.post') + def test_backoff_check_platform_access_connect_timeout_error(self, mock_post, mock_send, mock_request): + """ + Check whether the request backoffs properly for 120 seconds in case of ConnectTimeout error. + """ + mock_send.side_effect = ConnectTimeout + mock_post.side_effect = ConnectTimeout + before_time = datetime.datetime.now() + config = {"start_date": "dummy_st", "client_id": "dummy_ci", "client_secret": "dummy_cs", "tenant_id": "dummy_ti", "refresh_token": "dummy_rt"} + client = XeroClient(config) + client.access_token = "dummy_token" + with self.assertRaises(Timeout): + client.check_platform_access(config, "dummy_path") + after_time = datetime.datetime.now() + time_difference = (after_time - before_time).total_seconds() + self.assertGreaterEqual(time_difference, 120) + + @mock.patch('tap_xero.client.requests.Session.post') + def test_backoff_refresh_credentials_timeout_error(self, mock_post): + """ + Check whether the request backoffs properly for 60 seconds in case of Timeout error. + """ + mock_post.side_effect = Timeout + before_time = datetime.datetime.now() + config = {"start_date": "dummy_st", "client_id": "dummy_ci", "client_secret": "dummy_cs", "tenant_id": "dummy_ti", "refresh_token": "dummy_rt"} + client = XeroClient(config) + with self.assertRaises(Timeout): + client.refresh_credentials(config, "dummy_path") + after_time = datetime.datetime.now() + time_difference = (after_time - before_time).total_seconds() + self.assertGreaterEqual(time_difference, 60) + + @mock.patch('tap_xero.client.requests.Session.post') + def test_backoff_refresh_credentials_connect_timeout_error(self, mock_post): + """ + Check whether the request backoffs properly for 60 seconds in case of ConnectTimeout error. + """ + mock_post.side_effect = ConnectTimeout + before_time = datetime.datetime.now() + config = {"start_date": "dummy_st", "client_id": "dummy_ci", "client_secret": "dummy_cs", "tenant_id": "dummy_ti", "refresh_token": "dummy_rt"} + client = XeroClient(config) + with self.assertRaises(Timeout): + client.refresh_credentials(config, "dummy_path") + after_time = datetime.datetime.now() + time_difference = (after_time - before_time).total_seconds() + self.assertGreaterEqual(time_difference, 60) + + @mock.patch('tap_xero.client.requests.Session.send') + @mock.patch('tap_xero.client.requests.Request') + def test_backoff_filter_timeout_error(self, mock_request, mock_send): + """ + Check whether the request backoffs properly for 5 times in case of Timeout error. + """ + mock_send.side_effect = Timeout + before_time = datetime.datetime.now() + config = {"start_date": "dummy_st", "client_id": "dummy_ci", "client_secret": "dummy_cs", "tenant_id": "dummy_ti", "refresh_token": "dummy_rt"} + client = XeroClient(config) + client.access_token = "dummy_token" + with self.assertRaises(Timeout): + client.filter(tap_stream_id='dummy_stream') + after_time = datetime.datetime.now() + time_difference = (after_time - before_time).total_seconds() + self.assertGreaterEqual(time_difference, 60) + +class MockResponse(): + ''' + Mock response object for the requests call + ''' + def __init__(self, resp, status_code, content=[""], headers=None, raise_error=False, text={}): + self.json_data = resp + self.status_code = status_code + self.content = content + self.headers = headers + self.raise_error = raise_error + self.text = text + self.reason = "error" + + def prepare(self): + return (self.json_data, self.status_code, self.content, self.headers, self.raise_error) + + def json(self): + return self.text + +class MockRequest(): + ''' + Mock Request object for mocking the Requests() + ''' + def __init__(self): + pass + + def prepare(self): + pass + +class TestRequestTimeoutValue(unittest.TestCase): + ''' + Test that request timeout parameter works properly in various cases + ''' + @mock.patch('tap_xero.client.requests.Session.send', return_value = MockResponse("", status_code=200)) + @mock.patch('tap_xero.client.requests.Request', return_value = MockRequest()) + @mock.patch('tap_xero.client.XeroClient.refresh_credentials') + def test_config_provided_request_timeout(self, mock_refresh_creds, mock_request, mock_send): + """ + Unit tests to ensure that request timeout is set based on config value + """ + config = {"start_date": "dummy_st", "client_id": "dummy_ci", "client_secret": "dummy_cs", "tenant_id": "dummy_ti", "refresh_token": "dummy_rt", "request_timeout": 100} + client = XeroClient(config) + client.access_token = "dummy_access_token" + client.check_platform_access("GET", "dummy_path") + mock_send.assert_called_with(MockRequest().prepare(), timeout=100.0) + + @mock.patch('tap_xero.client.requests.Session.send', return_value = MockResponse("", status_code=200)) + @mock.patch('tap_xero.client.requests.Request', return_value = MockRequest()) + @mock.patch('tap_xero.client.XeroClient.refresh_credentials') + def test_default_value_request_timeout(self, mock_refresh_creds, mock_request, mock_send): + """ + Unit tests to ensure that request timeout is set based default value + """ + config = {"start_date": "dummy_st", "client_id": "dummy_ci", "client_secret": "dummy_cs", "tenant_id": "dummy_ti", "refresh_token": "dummy_rt"} + client = XeroClient(config) + client.access_token = "dummy_access_token" + client.check_platform_access("GET", "dummy_path") + mock_send.assert_called_with(MockRequest().prepare(), timeout=300.0) + + @mock.patch('tap_xero.client.requests.Session.send', return_value = MockResponse("", status_code=200)) + @mock.patch('tap_xero.client.requests.Request', return_value = MockRequest()) + @mock.patch('tap_xero.client.XeroClient.refresh_credentials') + def test_config_provided_empty_request_timeout(self, mock_refresh_creds, mock_request, mock_send): + """ + Unit tests to ensure that request timeout is set based on default value if empty value is given in config + """ + config = {"start_date": "dummy_st", "client_id": "dummy_ci", "client_secret": "dummy_cs", "tenant_id": "dummy_ti", "refresh_token": "dummy_rt", "request_timeout": ""} + client = XeroClient(config) + client.access_token = "dummy_access_token" + client.check_platform_access("GET", "dummy_path") + mock_send.assert_called_with(MockRequest().prepare(), timeout=300.0) + + @mock.patch('tap_xero.client.requests.Session.send', return_value = MockResponse("", status_code=200)) + @mock.patch('tap_xero.client.requests.Request', return_value = MockRequest()) + @mock.patch('tap_xero.client.XeroClient.refresh_credentials') + def test_config_provided_string_request_timeout(self, mock_refresh_creds, mock_request, mock_send): + """ + Unit tests to ensure that request timeout is set based on config string value + """ + config = {"start_date": "dummy_st", "client_id": "dummy_ci", "client_secret": "dummy_cs", "tenant_id": "dummy_ti", "refresh_token": "dummy_rt", "request_timeout": "100"} + client = XeroClient(config) + client.access_token = "dummy_access_token" + client.check_platform_access("GET", "dummy_path") + mock_send.assert_called_with(MockRequest().prepare(), timeout=100.0) + + @mock.patch('tap_xero.client.requests.Session.send', return_value = MockResponse("", status_code=200)) + @mock.patch('tap_xero.client.requests.Request', return_value = MockRequest()) + @mock.patch('tap_xero.client.XeroClient.refresh_credentials') + def test_config_provided_float_request_timeout(self, mock_refresh_creds, mock_request, mock_send): + """ + Unit tests to ensure that request timeout is set based on config float value + """ + config = {"start_date": "dummy_st", "client_id": "dummy_ci", "client_secret": "dummy_cs", "tenant_id": "dummy_ti", "refresh_token": "dummy_rt", "request_timeout": 100.8} + client = XeroClient(config) + client.access_token = "dummy_access_token" + client.check_platform_access("GET", "dummy_path") + mock_send.assert_called_with(MockRequest().prepare(), timeout=100.8) \ No newline at end of file