TDL-16368 add request timeout

.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'

README.md

@@ -167,11 +167,12 @@ This tap:
     {
         "access_token": "YOUR_ACCESS_TOKEN",
         "start_date": "2019-01-01T00:00:00Z",
-        "user_agent": "tap-recharge <api_user_email@your_company.com>"
+        "user_agent": "tap-recharge <api_user_email@your_company.com>",
+        "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
     {

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:

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,20 +102,34 @@ 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
 
     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):

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'})