Add custom timeout to TwilioHttpClient constructor (#485)

* Adds timeout to TwilioHttpClient constructor

* Adds unit tests and code alterations to cover various scenarios detailed in PR discussion

tests/unit/http/test_http_client.py

@@ -1,13 +1,10 @@
 # -*- coding: utf-8 -*-
-
 import six
 
 import unittest
 
-import mock
 from mock import patch, Mock
-from requests import Request
-from requests import Session
+from requests import Request, Session
 
 from twilio.http.http_client import TwilioHttpClient
 from twilio.http.response import Response
@@ -42,6 +39,61 @@ def test_request_sets_host_header_if_missing(self):
         self.assertIsNotNone(self.client.last_request)
         self.assertIsNotNone(self.client.last_response)
 
+    def test_request_with_timeout(self):
+        self.request_mock.url = 'https://api.twilio.com/'
+        self.request_mock.headers = {'Host': 'other.twilio.com'}
+
+        response = self.client.request(
+            'doesnt matter', 'doesnt matter', None, None, None, None, 30, None)
+
+        self.assertEqual('other.twilio.com', self.request_mock.headers['Host'])
+        self.assertEqual(200, response.status_code)
+        self.assertEqual('testing-unicode: Ω≈ç√, 💩', response.content)
+
+    def test_request_where_method_timeout_equals_zero(self):
+        self.request_mock.url = 'https://api.twilio.com/'
+        self.request_mock.headers = {'Host': 'other.twilio.com'}
+
+        try:
+            self.client.request(
+                'doesnt matter', 'doesnt matter', None, None, None, None, 0, None)
+        except Exception as e:
+            self.assertEqual(ValueError, type(e))
+
+    def test_request_where_class_timeout_manually_set(self):
+        self.request_mock.url = 'https://api.twilio.com/'
+        self.request_mock.headers = {'Host': 'other.twilio.com'}
+        self.client.timeout = 30
+
+        response = self.client.request(
+                'doesnt matter', 'doesnt matter')
+        self.assertEqual('other.twilio.com', self.request_mock.headers['Host'])
+        self.assertEqual(200, response.status_code)
+        self.assertEqual('testing-unicode: Ω≈ç√, 💩', response.content)
+
+    def test_request_where_class_timeout_equals_zero(self):
+        self.request_mock.url = 'https://api.twilio.com/'
+        self.request_mock.headers = {'Host': 'other.twilio.com'}
+        self.client.timeout = 0
+
+        try:
+            self.client.request(
+                'doesnt matter', 'doesnt matter')
+        except Exception as e:
+            self.assertEqual(type(e), ValueError)
+
+    def test_request_where_class_timeout_and_method_timeout_set(self):
+        self.request_mock.url = 'https://api.twilio.com/'
+        self.request_mock.headers = {'Host': 'other.twilio.com'}
+        self.client.timeout = 30
+
+        response = self.client.request(
+            'doesnt matter', 'doesnt matter', None, None, None, None, 15, None)
+
+        self.assertEqual('other.twilio.com', self.request_mock.headers['Host'])
+        self.assertEqual(200, response.status_code)
+        self.assertEqual('testing-unicode: Ω≈ç√, 💩', response.content)
+
     def test_request_with_unicode_response(self):
         self.request_mock.url = 'https://api.twilio.com/'
         self.request_mock.headers = {'Host': 'other.twilio.com'}

twilio/http/http_client.py

@@ -13,12 +13,24 @@ class TwilioHttpClient(HttpClient):
     """
     General purpose HTTP Client for interacting with the Twilio API
     """
-    def __init__(self, pool_connections=True, request_hooks=None):
+    def __init__(self, pool_connections=True, request_hooks=None, timeout=None):
+        """
+        Constructor for the TwilioHttpClient
+
+        :param bool pool_connections
+        :param request_hooks
+        :param int timeout: Timeout for the requests.
+                            Timeout should never be zero (0) or less.
+        """
         self.session = Session() if pool_connections else None
         self.last_request = None
         self.last_response = None
         self.request_hooks = request_hooks or hooks.default_hooks()
 
+        if timeout is not None and timeout <= 0:
+            raise ValueError(timeout)
+        self.timeout = timeout
+
     def request(self, method, url, params=None, data=None, headers=None, auth=None, timeout=None,
                 allow_redirects=False):
         """
@@ -37,6 +49,8 @@ def request(self, method, url, params=None, data=None, headers=None, auth=None,
         :return: An http response
         :rtype: A :class:`Response <twilio.rest.http.response.Response>` object
         """
+        if timeout is not None and timeout <= 0:
+            raise ValueError(timeout)
 
         kwargs = {
             'method': method.upper(),
@@ -65,7 +79,7 @@ def request(self, method, url, params=None, data=None, headers=None, auth=None,
         response = session.send(
             prepped_request,
             allow_redirects=allow_redirects,
-            timeout=timeout,
+            timeout=timeout if timeout is not None else self.timeout,
         )
 
         _logger.info('{method} Response: {status} {text}'.format(method=method, status=response.status_code, text=response.text))