From bb06d049c19652a594645396dbf9518e1994c941 Mon Sep 17 00:00:00 2001
From: Nicolas Le Manchet <nicolas.le-manchet@ovh.pl>
Date: Thu, 30 Jul 2015 10:25:05 +0200
Subject: [PATCH] Add timeout for requests

Default timeout is set to a sane value. It can be overridden by passing
'timeout' argument to the Client constructor.

Signed-off-by: Nicolas Le Manchet <nicolas.le-manchet@ovh.pl>
---
 ovh/client.py        | 18 ++++++++++++++++--
 tests/test_client.py | 18 ++++++++++++++----
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/ovh/client.py b/ovh/client.py
index 921e60f..5bdcbe0 100644
--- a/ovh/client.py
+++ b/ovh/client.py
@@ -67,6 +67,9 @@ ENDPOINTS = {
     'runabove-ca': 'https://api.runabove.com/1.0',
 }
 
+#: Default timeout for each request. 180 seconds connect, 180 seconds read.
+TIMEOUT = 180
+
 
 class Client(object):
     """
@@ -101,7 +104,7 @@ class Client(object):
     """
 
     def __init__(self, endpoint=None, application_key=None,
-                 application_secret=None, consumer_key=None):
+                 application_secret=None, consumer_key=None, timeout=TIMEOUT):
         """
         Creates a new Client. No credential check is done at this point.
 
@@ -117,10 +120,18 @@ class Client(object):
         See :py:mod:`ovh.config` for more informations on supported
         configuration mechanisms.
 
+        ``timeout`` can either be a float or a tuple. If it is a float it
+        sets the same timeout for both connection and read. If it is a tuple
+        connection and read timeout will be set independently. To use the
+        latter approach you need at least requests v2.4.0. Default value is
+        180 seconds for connection and 180 seconds for read.
+
         :param str endpoint: API endpoint to use. Valid values in ``ENDPOINTS``
         :param str application_key: Application key as provided by OVH
         :param str application_secret: Application secret key as provided by OVH
         :param str consumer_key: uniquely identifies
+        :param tuple timeout: Connection and read timeout for each request
+        :param float timeout: Same timeout for both connection and read
         :raises InvalidRegion: if ``endpoint`` can't be found in ``ENDPOINTS``.
         """
         # load endpoint
@@ -152,6 +163,9 @@ class Client(object):
         # use a requests session to reuse HTTPS connections between requests
         self._session = Session()
 
+        # Override default timeout
+        self._timeout = timeout
+
     ## high level API
 
     @property
@@ -374,7 +388,7 @@ class Client(object):
         # attempt request
         try:
             result = self._session.request(method, target, headers=headers,
-                                           data=body)
+                                           data=body, timeout=self._timeout)
         except RequestException as error:
             raise HTTPError("Low HTTP request failed error", error)
 
diff --git a/tests/test_client.py b/tests/test_client.py
index d288fc7..47bb6e9 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -63,6 +63,8 @@ FAKE_TIME = 1404395889.467238
 FAKE_METHOD = 'MeThOd'
 FAKE_PATH = '/unit/test'
 
+TIMEOUT = 180
+
 class testClient(unittest.TestCase):
     def setUp(self):
         self.time_patch = mock.patch('time.time', return_value=FAKE_TIME)
@@ -80,6 +82,13 @@ class testClient(unittest.TestCase):
         self.assertEqual(APPLICATION_SECRET, api._application_secret)
         self.assertEqual(CONSUMER_KEY, api._consumer_key)
         self.assertTrue(api._time_delta is None)
+        self.assertEqual(TIMEOUT, api._timeout)
+
+        # override default timeout
+        timeout = (1, 1)
+        api = Client(ENDPOINT, APPLICATION_KEY, APPLICATION_SECRET,
+                     CONSUMER_KEY, timeout=timeout)
+        self.assertEqual(timeout, api._timeout)
 
         # invalid region
         self.assertRaises(InvalidRegion, Client, ENDPOINT_BAD, '', '', '')
@@ -212,7 +221,8 @@ class testClient(unittest.TestCase):
         self.assertEqual(m_json, api.call(FAKE_METHOD, FAKE_PATH, None, False))
         m_req.assert_called_once_with(
             FAKE_METHOD, BASE_URL+'/unit/test',
-            headers={'X-Ovh-Application': APPLICATION_KEY}, data=''
+            headers={'X-Ovh-Application': APPLICATION_KEY}, data='',
+            timeout=TIMEOUT
         )
         m_req.reset_mock()
 
@@ -226,7 +236,7 @@ class testClient(unittest.TestCase):
             headers={
                 'X-Ovh-Application': APPLICATION_KEY,
                 'Content-type': 'application/json',
-            }, data=j_data
+            }, data=j_data, timeout=TIMEOUT
         )
         m_req.reset_mock()
 
@@ -289,7 +299,7 @@ class testClient(unittest.TestCase):
                 'X-Ovh-Application': APPLICATION_KEY,
                 'X-Ovh-Signature': '$1$16ae5ba8c63841b1951575be905867991d5f49dc',
                 'X-Ovh-Timestamp': '1404395931',
-            }, data=''
+            }, data='', timeout=TIMEOUT
         )
         m_time_delta.reset_mock()
         m_req.reset_mock()
@@ -308,7 +318,7 @@ class testClient(unittest.TestCase):
                 'Content-type': 'application/json',
                 'X-Ovh-Timestamp': '1404395931',
                 'X-Ovh-Signature': '$1$9acb1ac0120006d16261a635aed788e83ab172d2',
-                }, data=json.dumps(data)
+                }, data=json.dumps(data), timeout=TIMEOUT
         )
         m_time_delta.reset_mock()
         m_req.reset_mock()