TDL-16355 Implement request timeouts

.circleci/config.yml

@@ -12,6 +12,7 @@ jobs:
             source /usr/local/share/virtualenvs/tap-mailchimp/bin/activate
             pip install -U 'pip<19.2' 'setuptools<51.0.0'
             pip install .[dev]
+            pip install coverage
       - run:
           name: 'pylint'
           command: |
@@ -27,7 +28,12 @@ jobs:
           name: 'Unit Tests'
           command: |
             source /usr/local/share/virtualenvs/tap-mailchimp/bin/activate
-            nosetests ./tests/unittests
+            nosetests --with-coverage --cover-erase --cover-package=tap_mailchimp --cover-html-dir=htmlcov ./tests/unittests
+            coverage html
+      - store_test_results:
+          path: test_output/report.xml
+      - store_artifacts:
+          path: htmlcov
       - run:
           name: 'Integration Tests'
           command: |

README.md

@@ -30,6 +30,7 @@ Config properties:
 | `dc` | See note. | "us14" | The Mailchimp data center, only requried when using API key auth. |
 | `start_date` | Y | "2010-01-01T00:00:00Z" | The default start date to use for date modified replication, when available. |
 | `user_agent` | N | "Vandelay Industries ETL Runner" | The user agent to send on every request. |
+| `request_timeout` | N | 300 | Time for which request should wait to get response. |
 
 ## Usage 
 

tap_mailchimp/client.py

@@ -1,11 +1,12 @@
 import backoff
 import requests
 import singer
-from requests.exceptions import ConnectionError # pylint: disable=redefined-builtin
+from requests.exceptions import ConnectionError, Timeout # pylint: disable=redefined-builtin
 from singer import metrics
 
 LOGGER = singer.get_logger()
 
+REQUEST_TIMEOUT = 300
 class ClientRateLimitError(Exception):
     pass
 
@@ -21,6 +22,13 @@ def __init__(self, config):
         self.__base_url = None
         self.page_size = int(config.get('page_size', '1000'))
 
+        # Set request timeout to config param `request_timeout` value.
+        # If value is 0,"0","" or not passed then it set default to 300 seconds.
+        config_request_timeout = config.get('request_timeout')
+        if config_request_timeout and float(config_request_timeout):
+            self.__request_timeout = float(config_request_timeout)
+        else:
+            self.__request_timeout = REQUEST_TIMEOUT
 
         if not self.__access_token and self.__api_key:
             self.__base_url = 'https://{}.api.mailchimp.com'.format(
@@ -38,6 +46,10 @@ def get_base_url(self):
                             endpoint='base_url')
         self.__base_url = data['api_endpoint']
 
+    @backoff.on_exception(backoff.expo,
+                          Timeout, # Backoff for request timeout
+                          max_tries=5,
+                          factor=2)
     @backoff.on_exception(backoff.expo,
                           (Server5xxError, ClientRateLimitError, ConnectionError),
                           max_tries=6,
@@ -74,7 +86,7 @@ def request(self, method, path=None, url=None, s3=False, **kwargs):
 
         with metrics.http_request_timer(endpoint) as timer:
             LOGGER.info("Executing %s request to %s with params: %s", method, url, kwargs.get('params'))
-            response = self.__session.request(method, url, **kwargs)
+            response = self.__session.request(method, url, timeout=self.__request_timeout, **kwargs) # Pass request timeout
             timer.tags[metrics.Tag.http_status_code] = response.status_code
 
         if response.status_code >= 500:

tests/unittests/test_request_timeouts.py

@@ -0,0 +1,134 @@
+from tap_mailchimp.client import MailchimpClient
+import unittest
+from unittest.mock import patch
+import requests
+
+REQUEST_TIMEOUT_INT = 300
+REQUEST_TIMEOUT_STR = "300"
+REQUEST_TIMEOUT_FLOAT = 300.0
+
+# Mock response object
+def get_mock_http_response(*args, **kwargs):
+    contents = '{"access_token": "test", "expires_in":100, "accounts":[{"id": 12}]}'
+    response = requests.Response()
+    response.status_code = 200
+    response._content = contents.encode()
+    return response
+
+@patch("time.sleep")
+@patch("requests.Session.request", side_effect=requests.exceptions.Timeout)
+class TestRequestTimeoutsBackoff(unittest.TestCase):
+
+    def test_no_request_timeout_in_config(self, mocked_request, mock_sleep):
+        """
+            Verify that if request_timeout is not provided in config then default value is used
+        """
+        # Initialize MailchimpClient object
+        client = MailchimpClient({'access_token': 'as'})
+        try:
+            client.request('GET', "http://test", "base_url")
+        except requests.exceptions.Timeout:
+            pass
+
+        # Verify that requests.Session.request is called 5 times
+        self.assertEqual(mocked_request.call_count, 5)
+
+@patch("requests.Session.request", side_effect=get_mock_http_response)
+class TestRequestTimeoutsValue(unittest.TestCase):
+
+    def test_no_request_timeout_in_config(self, mocked_request):
+        """
+            Verify that if request_timeout is not provided in config then default value is used
+        """
+        # Initialize MailchimpClient object
+        client = MailchimpClient({'access_token': 'as'})
+
+        # Call request method which call requests.Session.request with timeout
+        client.request('GET', "http://test", "base_url")
+
+        # Verify requests.Session.request is called with expected timeout
+        args, kwargs = mocked_request.call_args
+        self.assertEqual(kwargs.get('timeout'), REQUEST_TIMEOUT_INT) # Verify timeout argument
+
+    def test_integer_request_timeout_in_config(self, mocked_request):
+        """
+            Verify that if request_timeout is provided in config(integer value) then it should be use
+        """
+        # Initialize MailchimpClient object
+        client = MailchimpClient({'access_token': 'as', "request_timeout": REQUEST_TIMEOUT_INT}) # integer timeout in config
+
+        # Call request method which call requests.Session.request with timeout
+        client.request('GET', "http://test", "base_url")
+
+        # Verify requests.Session.request is called with expected timeout
+        args, kwargs = mocked_request.call_args
+        self.assertEqual(kwargs.get('timeout'), REQUEST_TIMEOUT_FLOAT) # Verify timeout argument
+
+    def test_float_request_timeout_in_config(self, mocked_request):
+        """
+            Verify that if request_timeout is provided in config(float value) then it should be use
+        """
+        # Initialize MailchimpClient object
+        client = MailchimpClient({'access_token': 'as', "request_timeout": REQUEST_TIMEOUT_FLOAT}) # float timeout in config
+
+        # Call request method which call requests.Session.request with timeout
+        client.request('GET', "http://test", "base_url")
+
+        # Verify requests.Session.request is called with expected timeout
+        args, kwargs = mocked_request.call_args
+        self.assertEqual(kwargs.get('timeout'), REQUEST_TIMEOUT_FLOAT) # Verify timeout argument
+
+    def test_string_request_timeout_in_config(self, mocked_request):
+        """
+            Verify that if request_timeout is provided in config(string value) then it should be use
+        """
+        # Initialize MailchimpClient object
+        client = MailchimpClient({'access_token': 'as', "request_timeout": REQUEST_TIMEOUT_STR}) # string timeout in config
+
+        # Call request method which call requests.Session.request with timeout
+        client.request('GET', "http://test", "base_url")
+
+        # Verify requests.Session.request is called with expected timeout
+        args, kwargs = mocked_request.call_args
+        self.assertEqual(kwargs.get('timeout'), REQUEST_TIMEOUT_FLOAT) # Verify timeout argument
+
+    def test_empty_string_request_timeout_in_config(self, mocked_request):
+        """
+            Verify that if request_timeout is provided in config with empty string then default value is used
+        """
+        # Initialize MailchimpClient object
+        client = MailchimpClient({'access_token': 'as', "request_timeout": ""}) # empty string timeout in config
+
+        # Call request method which call requests.Session.request with timeout
+        client.request('GET', "http://test", "base_url")
+
+        # Verify requests.Session.request is called with expected timeout
+        args, kwargs = mocked_request.call_args
+        self.assertEqual(kwargs.get('timeout'), REQUEST_TIMEOUT_INT) # Verify timeout argument
+
+    def test_zero_int_request_timeout_in_config(self, mocked_request):
+        """
+            Verify that if request_timeout is provided in config with int zero value then default value is used
+        """
+        # Initialize MailchimpClient object
+        client = MailchimpClient({'access_token': 'as', "request_timeout": 0}) # int zero value in config
+
+        # Call request method which call requests.Session.request with timeout
+        client.request('GET', "http://test", "base_url")
+
+        # Verify requests.Session.request is called with expected timeout
+        args, kwargs = mocked_request.call_args
+        self.assertEqual(kwargs.get('timeout'), REQUEST_TIMEOUT_INT) # Verify timeout argument
+
+    def test_zero_string_request_timeout_in_config(self, mocked_request):
+        """
+            Verify that if request_timeout is provided in config with string zero in string format then default value is used
+        """
+        client = MailchimpClient({'access_token': 'as', "request_timeout": "0"}) # string zero value in config
+
+        # Call request method which call requests.Session.request with timeout
+        client.request('GET', "http://test", "base_url")
+
+        # Verify requests.Session.request is called with expected timeout
+        args, kwargs = mocked_request.call_args
+        self.assertEqual(kwargs.get('timeout'), REQUEST_TIMEOUT_INT) # Verify timeout argument