diff --git a/.circleci/config.yml b/.circleci/config.yml index 307b36d..365f9bb 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -15,7 +15,18 @@ jobs: name: 'pylint' command: | source /usr/local/share/virtualenvs/tap-recharge/bin/activate - pylint tap_recharge --disable 'missing-module-docstring,missing-function-docstring,missing-class-docstring,no-else-raise,raise-missing-from,inconsistent-return-statements' + pylint tap_recharge --disable 'missing-module-docstring,missing-function-docstring,missing-class-docstring,no-else-raise,raise-missing-from,inconsistent-return-statements,line-too-long' + - run: + name: 'Unit Tests' + command: | + source /usr/local/share/virtualenvs/tap-recharge/bin/activate + pip install nose coverage + nosetests --with-coverage --cover-erase --cover-package=tap_recharge --cover-html-dir=htmlcov tests/unittests + coverage html + - store_test_results: + path: test_output/report.xml + - store_artifacts: + path: htmlcov - run: when: always name: 'Integration Tests' diff --git a/README.md b/README.md index 1d183a8..b98fe3f 100644 --- a/README.md +++ b/README.md @@ -167,11 +167,12 @@ This tap: { "access_token": "YOUR_ACCESS_TOKEN", "start_date": "2019-01-01T00:00:00Z", - "user_agent": "tap-recharge " + "user_agent": "tap-recharge ", + "request_timeout": 300 } ``` - Optionally, also create a `state.json` file. `currently_syncing` is an optional attribute used for identifying the last object to be synced in case the job is interrupted mid-stream. The next run would begin where the last job left off. + Optionally, also create a `state.json` file. `currently_syncing` is an optional attribute used for identifying the last object to be synced in case the job is interrupted mid-stream. The next run would begin where the last job left off. The `request_timeout` is an optional parameter to set a timeout for requests. Default: 300 seconds ```json { diff --git a/tap_recharge/__init__.py b/tap_recharge/__init__.py index dfdaf9f..e0e252f 100644 --- a/tap_recharge/__init__.py +++ b/tap_recharge/__init__.py @@ -30,7 +30,9 @@ def main(): with RechargeClient( parsed_args.config['access_token'], - parsed_args.config['user_agent']) as client: + parsed_args.config['user_agent'], + parsed_args.config.get('request_timeout') + ) as client: state = {} if parsed_args.state: diff --git a/tap_recharge/client.py b/tap_recharge/client.py index 108f65f..d6c45e1 100644 --- a/tap_recharge/client.py +++ b/tap_recharge/client.py @@ -3,9 +3,10 @@ import singer from singer import metrics, utils +from requests.exceptions import Timeout LOGGER = singer.get_logger() - +REQUEST_TIMEOUT = 300 class Server5xxError(Exception): pass @@ -101,13 +102,26 @@ class RechargeClient: def __init__( self, access_token, - user_agent=None): + user_agent=None, + request_timeout=REQUEST_TIMEOUT): self.__access_token = access_token self.__user_agent = user_agent self.__session = requests.Session() self.__base_url = None self.__verified = False - + # 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 the request for 5 times when Timeout or Connection error occurs + @backoff.on_exception( + backoff.expo, + (Timeout, requests.ConnectionError), + max_tries=5, + factor=2) def __enter__(self): self.__verified = self.check_access_token() return self @@ -115,6 +129,7 @@ def __enter__(self): def __exit__(self, exception_type, exception_value, traceback): self.__session.close() + # Backoff the request for 5 times when Timeout error occurs @backoff.on_exception( backoff.expo, Server5xxError, @@ -131,17 +146,18 @@ def check_access_token(self): response = self.__session.get( # Simple endpoint that returns 1 record w/ default organization URN url='https://api.rechargeapps.com', - headers=headers) + headers=headers, + timeout=self.request_timeout) if response.status_code != 200: LOGGER.error('Error status_code = %s', response.status_code) raise_for_error(response) else: return True - + # Added backoff for 5 times when Timeout error occurs @backoff.on_exception( backoff.expo, - (Server5xxError, requests.ConnectionError, Server429Error), + (Timeout, Server5xxError, requests.ConnectionError, Server429Error), max_tries=5, factor=2) # Call/rate limit: https://developer.rechargepayments.com/#rate-limits @@ -174,7 +190,7 @@ def request(self, method, path=None, url=None, **kwargs): kwargs['headers']['Content-Type'] = 'application/json' with metrics.http_request_timer(endpoint) as timer: - response = self.__session.request(method, url, stream=True, **kwargs) + response = self.__session.request(method, url, stream=True, timeout=self.request_timeout, **kwargs) timer.tags[metrics.Tag.http_status_code] = response.status_code if response.status_code >= 500: @@ -192,7 +208,6 @@ def request(self, method, path=None, url=None, **kwargs): except Exception as err: LOGGER.error(err) raise Exception(err) - return response_json, response.links def get(self, path, **kwargs): diff --git a/tests/unittests/test_request_timeout.py b/tests/unittests/test_request_timeout.py new file mode 100644 index 0000000..5f50c68 --- /dev/null +++ b/tests/unittests/test_request_timeout.py @@ -0,0 +1,150 @@ +from tap_recharge.client import RechargeClient +import unittest +from unittest import mock +from requests.exceptions import Timeout, ConnectionError + +class TestBackoffError(unittest.TestCase): + ''' + Test that backoff logic works properly. + ''' + @mock.patch('tap_recharge.client.requests.Session.request') + @mock.patch('tap_recharge.client.RechargeClient.check_access_token') + def test_request_timeout_and_backoff(self, mock_get_token, mock_request): + """ + Check whether the request backoffs properly for request() for 5 times in case of Timeout error. + """ + mock_request.side_effect = Timeout + client = RechargeClient("dummy_access_token", "dummy_user_agent", 300) + with self.assertRaises(Timeout): + client.request("GET") + self.assertEquals(mock_request.call_count, 5) + + @mock.patch('tap_recharge.client.requests.Session.request') + def test_check_access_token_timeout_and_backoff(self, mocked_request): + """ + Check whether the request backoffs properly for __enter__() for 5 times in case of Timeout error. + """ + mocked_request.side_effect = Timeout + + config = { + "access_token": "dummy_at", + "user_agent": "dummy_ua" + } + # initialize 'RechargeClient' + try: + with RechargeClient( + config['access_token'], + config['user_agent'], + config.get('request_timeout')) as client: + pass + except Timeout: + pass + # verify that we backoff for 5 times + self.assertEquals(mocked_request.call_count, 5) + + @mock.patch('tap_recharge.client.requests.Session.request') + def test_check_access_token_connection_error_and_backoff(self, mocked_request): + """ + Check whether the request backoffs properly for __enter__() for 5 times in case of Timeout error. + """ + mocked_request.side_effect = ConnectionError + + config = { + "access_token": "dummy_at", + "user_agent": "dummy_ua" + } + # initialize 'RechargeClient' + try: + with RechargeClient( + config['access_token'], + config['user_agent'], + config.get('request_timeout')) as client: + pass + except ConnectionError: + pass + # verify that we backoff for 5 times + self.assertEquals(mocked_request.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={}, links=""): + 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" + self.links = links + + def prepare(self): + return (self.json_data, self.status_code, self.content, self.headers, self.raise_error) + + def json(self): + return self.text + +class TestRequestTimeoutValue(unittest.TestCase): + ''' + Test that request timeout parameter works properly in various cases + ''' + @mock.patch('tap_recharge.client.requests.Session.request', return_value = MockResponse("", status_code=200)) + @mock.patch('tap_recharge.client.RechargeClient.check_access_token') + def test_config_provided_request_timeout(self, mock_get, mock_request): + """ + Unit tests to ensure that request timeout is set based on config value + """ + config = {"access_token": "dummy_at", "user_agent": "dummy_ua", "request_timeout": 100} + client = RechargeClient(**config) + client.request("GET", "dummy_path") + + mock_request.assert_called_with('GET', 'https://api.rechargeapps.com/dummy_path', stream=True, timeout=100.0, headers={'X-Recharge-Access-Token': 'dummy_at', 'Accept': 'application/json', 'User-Agent': 'dummy_ua'}) + + @mock.patch('tap_recharge.client.requests.Session.request', return_value = MockResponse("", status_code=200)) + @mock.patch('tap_recharge.client.RechargeClient.check_access_token') + def test_default_value_request_timeout(self, mock_get, mock_request): + """ + Unit tests to ensure that request timeout is set based default value + """ + config = {"access_token": "dummy_at", "user_agent": "dummy_ua"} + client = RechargeClient(**config) + client.request("GET", "dummy_path") + + mock_request.assert_called_with('GET', 'https://api.rechargeapps.com/dummy_path', stream=True, timeout=300, headers={'X-Recharge-Access-Token': 'dummy_at', 'Accept': 'application/json', 'User-Agent': 'dummy_ua'}) + + @mock.patch('tap_recharge.client.requests.Session.request', return_value = MockResponse("", status_code=200)) + @mock.patch('tap_recharge.client.RechargeClient.check_access_token') + def test_config_provided_empty_request_timeout(self, mock_get, mock_request): + """ + Unit tests to ensure that request timeout is set based on default value if empty value is given in config + """ + config = {"access_token": "dummy_at", "user_agent": "dummy_ua", "request_timeout": ""} + client = RechargeClient(**config) + client.request("GET", "dummy_path") + + mock_request.assert_called_with('GET', 'https://api.rechargeapps.com/dummy_path', stream=True, timeout=300.0, headers={'X-Recharge-Access-Token': 'dummy_at', 'Accept': 'application/json', 'User-Agent': 'dummy_ua'}) + + @mock.patch('tap_recharge.client.requests.Session.request', return_value = MockResponse("", status_code=200)) + @mock.patch('tap_recharge.client.RechargeClient.check_access_token') + def test_config_provided_string_request_timeout(self, mock_get, mock_request): + """ + Unit tests to ensure that request timeout is set based on config string value + """ + config = {"access_token": "dummy_at", "user_agent": "dummy_ua", "request_timeout": "100"} + client = RechargeClient(**config) + client.request("GET", "dummy_path") + + mock_request.assert_called_with('GET', 'https://api.rechargeapps.com/dummy_path', stream=True, timeout=100.0, headers={'X-Recharge-Access-Token': 'dummy_at', 'Accept': 'application/json', 'User-Agent': 'dummy_ua'}) + + @mock.patch('tap_recharge.client.requests.Session.request', return_value = MockResponse("", status_code=200)) + @mock.patch('tap_recharge.client.RechargeClient.check_access_token') + def test_config_provided_float_request_timeout(self, mock_get, mock_request): + """ + Unit tests to ensure that request timeout is set based on config float value + """ + config = {"access_token": "dummy_at", "user_agent": "dummy_ua", "request_timeout": 100.8} + client = RechargeClient(**config) + client.request("GET", "dummy_path") + + mock_request.assert_called_with('GET', 'https://api.rechargeapps.com/dummy_path', stream=True, timeout=100.8, headers={'X-Recharge-Access-Token': 'dummy_at', 'Accept': 'application/json', 'User-Agent': 'dummy_ua'})