From fa74413284a817d49699f2cd6e98c832862932ab Mon Sep 17 00:00:00 2001 From: namrata270998 Date: Fri, 12 Nov 2021 04:39:21 +0000 Subject: [PATCH 1/9] addd request timeout --- tap_xero/client.py | 28 +++- tests/unittests/test_request_timeout.py | 189 ++++++++++++++++++++++++ 2 files changed, 212 insertions(+), 5 deletions(-) create mode 100644 tests/unittests/test_request_timeout.py diff --git a/tap_xero/client.py b/tap_xero/client.py index 2103739..462d464 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, ConnectTimeout 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.expo, (Timeout, ConnectTimeout), max_time=60, factor=2) 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.expo, Timeout, max_time=60, factor=2) @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 contacts_url = join(BASE_URL, "Contacts") request = requests.Request("GET", contacts_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 for 5 times in case of Timeout error + @backoff.on_exception(backoff.expo, Timeout, max_tries=5) @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..b7afb3b --- /dev/null +++ b/tests/unittests/test_request_timeout.py @@ -0,0 +1,189 @@ +from tap_xero.client import XeroClient +import unittest +from unittest import mock +from unittest.case import TestCase +from requests.exceptions import Timeout, ConnectTimeout +import datetime +from tap_xero import LOGGER + +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 60 seconds in case of Timeout error. + """ + mock_send.side_effect = Timeout + mock_post.side_effect = Timeout + before_time = datetime.datetime.now() + with self.assertRaises(Timeout): + 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.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 60 seconds in case of ConnectTimeout error. + """ + mock_send.side_effect = ConnectTimeout + mock_post.side_effect = ConnectTimeout + before_time = datetime.datetime.now() + with self.assertRaises(Timeout): + 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.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() + with self.assertRaises(Timeout): + 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.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() + with self.assertRaises(Timeout): + 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.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 + with self.assertRaises(Timeout): + 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" + client.filter(tap_stream_id='dummy_stream') + self.assertGreaterEqual(mock_send.call_count, 5) + +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 From c6eaa298d9b33e8fac2aede1af937887541d63e1 Mon Sep 17 00:00:00 2001 From: namrata270998 Date: Fri, 12 Nov 2021 05:00:14 +0000 Subject: [PATCH 2/9] fixed trailing spaces pylint errors --- tap_xero/client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index 462d464..d0376a8 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -193,7 +193,7 @@ def __init__(self, config): request_timeout = REQUEST_TIMEOUT self.request_timeout = request_timeout - # backoff for 1 minute when the Timeout error occurs as the request will again backoff + # 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.expo, (Timeout, ConnectTimeout), max_time=60, factor=2) @@ -223,7 +223,7 @@ 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 + # 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.expo, Timeout, max_time=60, factor=2) From c556d4c18010a5ec2efc623f58f5ed260e04f80c Mon Sep 17 00:00:00 2001 From: namrata270998 Date: Fri, 12 Nov 2021 05:42:00 +0000 Subject: [PATCH 3/9] fixed pylint errors --- .circleci/config.yml | 2 +- tests/unittests/test_request_timeout.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 09f857b..970788f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -21,7 +21,7 @@ jobs: name: 'pylint' command: | source /usr/local/share/virtualenvs/tap-xero/bin/activate - pylint tap_xero --disable 'broad-except,chained-comparison,empty-docstring,fixme,invalid-name,line-too-long,missing-class-docstring,missing-function-docstring,missing-module-docstring,no-else-raise,no-else-return,too-few-public-methods,too-many-arguments,too-many-branches,too-many-lines,too-many-locals,ungrouped-imports,wrong-spelling-in-comment,wrong-spelling-in-docstring,bad-whitespace,unspecified-encoding' + pylint tap_xero --disable 'broad-except,chained-comparison,empty-docstring,fixme,invalid-name,line-too-long,missing-class-docstring,missing-function-docstring,missing-module-docstring,no-else-raise,no-else-return,too-few-public-methods,too-many-arguments,too-many-branches,too-many-lines,too-many-locals,ungrouped-imports,wrong-spelling-in-comment,wrong-spelling-in-docstring,bad-whitespace,unspecified-encoding,consider-using-f-string' - run: name: 'Unit Tests' command: | diff --git a/tests/unittests/test_request_timeout.py b/tests/unittests/test_request_timeout.py index b7afb3b..8828946 100644 --- a/tests/unittests/test_request_timeout.py +++ b/tests/unittests/test_request_timeout.py @@ -4,7 +4,6 @@ from unittest.case import TestCase from requests.exceptions import Timeout, ConnectTimeout import datetime -from tap_xero import LOGGER class TestBackoffError(unittest.TestCase): ''' From 21a1865de3ae37fc4a1705af59ed5cb0f518fd83 Mon Sep 17 00:00:00 2001 From: namrata270998 Date: Fri, 12 Nov 2021 09:43:07 +0000 Subject: [PATCH 4/9] added coverage in cci --- .circleci/config.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 970788f..e0691a6 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: | From 57f8a469d5ff70fa696e27008c81383c7cce8740 Mon Sep 17 00:00:00 2001 From: namrata270998 Date: Fri, 12 Nov 2021 13:16:49 +0000 Subject: [PATCH 5/9] updated request_timeout --- tap_xero/client.py | 8 ++++---- tests/unittests/test_request_timeout.py | 5 ++++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index d0376a8..d7364f7 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -196,7 +196,7 @@ def __init__(self, config): # 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.expo, (Timeout, ConnectTimeout), max_time=60, factor=2) + @backoff.on_exception(backoff.constant, (Timeout, ConnectTimeout), max_time=60, interval=10) def refresh_credentials(self, config, config_path): header_token = b64encode((config["client_id"] + ":" + config["client_secret"]).encode('utf-8')) @@ -226,7 +226,7 @@ def refresh_credentials(self, config, config_path): # 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.expo, Timeout, max_time=60, factor=2) + @backoff.on_exception(backoff.constant, (Timeout, ConnectTimeout), max_time=60, interval=10) @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): @@ -249,8 +249,8 @@ def check_platform_access(self, config, config_path): raise_for_error(response) - # backoff for 5 times in case of Timeout error - @backoff.on_exception(backoff.expo, Timeout, max_tries=5) + # backoff till 60seconds in case of Timeout error + @backoff.on_exception(backoff.expo, (Timeout, ConnectTimeout), max_time=60, interval=10) @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): diff --git a/tests/unittests/test_request_timeout.py b/tests/unittests/test_request_timeout.py index 8828946..d0c7d40 100644 --- a/tests/unittests/test_request_timeout.py +++ b/tests/unittests/test_request_timeout.py @@ -82,12 +82,15 @@ 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() with self.assertRaises(Timeout): 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" client.filter(tap_stream_id='dummy_stream') - self.assertGreaterEqual(mock_send.call_count, 5) + after_time = datetime.datetime.now() + time_difference = (after_time - before_time).total_seconds() + self.assertGreaterEqual(time_difference, 60) class MockResponse(): ''' From e1b2739d68b55865624cf3f193e761fa619c3581 Mon Sep 17 00:00:00 2001 From: namrata270998 Date: Mon, 15 Nov 2021 09:31:30 +0000 Subject: [PATCH 6/9] updated comments --- tap_xero/client.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index d7364f7..350ba98 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -193,10 +193,10 @@ def __init__(self, config): request_timeout = REQUEST_TIMEOUT self.request_timeout = request_timeout - # backoff for 1 minute when the Timeout error occurs as the request will again backoff + # 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, ConnectTimeout), max_time=60, interval=10) + @backoff.on_exception(backoff.constant, Timeout, max_time=60, interval=10) def refresh_credentials(self, config, config_path): header_token = b64encode((config["client_id"] + ":" + config["client_secret"]).encode('utf-8')) @@ -223,10 +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 + # 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, ConnectTimeout), max_time=60, interval=10) + @backoff.on_exception(backoff.constant, Timeout, max_time=60, interval=10) @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): @@ -249,8 +249,8 @@ def check_platform_access(self, config, config_path): raise_for_error(response) - # backoff till 60seconds in case of Timeout error - @backoff.on_exception(backoff.expo, (Timeout, ConnectTimeout), max_time=60, interval=10) + # 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) @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): From 8795a490d1d1d1963eed3ec240d9c8c0bff759ec Mon Sep 17 00:00:00 2001 From: namrata270998 Date: Tue, 16 Nov 2021 13:22:46 +0000 Subject: [PATCH 7/9] added jitter --- tap_xero/client.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index 350ba98..9169fb9 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -196,7 +196,7 @@ def __init__(self, config): # 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) + @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')) @@ -226,7 +226,7 @@ def refresh_credentials(self, config, config_path): # 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) + @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): @@ -250,7 +250,7 @@ def check_platform_access(self, config, config_path): # 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) + @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): From 0e2790e7f8780ed2a10aa59c6759ce84ee778978 Mon Sep 17 00:00:00 2001 From: namrata270998 Date: Wed, 17 Nov 2021 12:46:09 +0000 Subject: [PATCH 8/9] removed connecttimeout --- tap_xero/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index 9169fb9..74d1abd 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -12,7 +12,7 @@ import pytz import backoff import singer -from requests.exceptions import Timeout, ConnectTimeout +from requests.exceptions import Timeout LOGGER = singer.get_logger() From 5141f962ad1292d890879d23db4dfa1fe11d466e Mon Sep 17 00:00:00 2001 From: namrata270998 Date: Wed, 1 Dec 2021 15:54:09 +0530 Subject: [PATCH 9/9] resolved comments --- tests/unittests/test_request_timeout.py | 29 +++++++++++++------------ 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/tests/unittests/test_request_timeout.py b/tests/unittests/test_request_timeout.py index d0c7d40..2016532 100644 --- a/tests/unittests/test_request_timeout.py +++ b/tests/unittests/test_request_timeout.py @@ -1,7 +1,6 @@ from tap_xero.client import XeroClient import unittest from unittest import mock -from unittest.case import TestCase from requests.exceptions import Timeout, ConnectTimeout import datetime @@ -14,14 +13,15 @@ class TestBackoffError(unittest.TestCase): @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 60 seconds in case of Timeout error. + 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): - 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.check_platform_access(config, "dummy_path") after_time = datetime.datetime.now() time_difference = (after_time - before_time).total_seconds() @@ -32,14 +32,15 @@ def test_backoff_check_platform_access_timeout_error(self, mock_post, mock_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 60 seconds in case of ConnectTimeout error. + 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): - 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.check_platform_access(config, "dummy_path") after_time = datetime.datetime.now() time_difference = (after_time - before_time).total_seconds() @@ -52,9 +53,9 @@ def test_backoff_refresh_credentials_timeout_error(self, mock_post): """ 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): - 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.refresh_credentials(config, "dummy_path") after_time = datetime.datetime.now() time_difference = (after_time - before_time).total_seconds() @@ -67,9 +68,9 @@ def test_backoff_refresh_credentials_connect_timeout_error(self, mock_post): """ 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): - 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.refresh_credentials(config, "dummy_path") after_time = datetime.datetime.now() time_difference = (after_time - before_time).total_seconds() @@ -83,10 +84,10 @@ def test_backoff_filter_timeout_error(self, mock_request, mock_send): """ 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): - 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" client.filter(tap_stream_id='dummy_stream') after_time = datetime.datetime.now() time_difference = (after_time - before_time).total_seconds()