[390136] Add timeout options to Armada client

Allows for timeouts to be specified for each Armada client call,
and sets a more reasonable default for connect timeout of 20 seconds
instead of 3600 seconds.  Read timeout default remains at 3600s

Change-Id: I6bb04a6c8d55db4b98310f62ea6f037b3efdde24

armada/common/client.py

@@ -33,25 +33,25 @@ class ArmadaClient(object):
     def _set_endpoint(self, version, action):
         return API_VERSION.format(version, action)
 
-    def get_status(self, query):
+    def get_status(self, query, timeout=None):
 
         endpoint = self._set_endpoint('1.0', 'status')
-        resp = self.session.get(endpoint, query=query)
+        resp = self.session.get(endpoint, query=query, timeout=timeout)
 
         self._check_response(resp)
 
         return resp.json()
 
-    def get_releases(self, query):
+    def get_releases(self, query, timeout=None):
 
         endpoint = self._set_endpoint('1.0', 'releases')
-        resp = self.session.get(endpoint, query=query)
+        resp = self.session.get(endpoint, query=query, timeout=timeout)
 
         self._check_response(resp)
 
         return resp.json()
 
-    def post_validate(self, manifest=None):
+    def post_validate(self, manifest=None, timeout=None):
 
         endpoint = self._set_endpoint('1.0', 'validatedesign')
         # TODO(sh8121att) Look to update the UCP convention to
@@ -63,7 +63,8 @@ class ArmadaClient(object):
             data=req_body,
             headers={
                 'content-type': 'application/json'
-            })
+            },
+            timeout=timeout)
 
         self._check_response(resp)
 
@@ -74,7 +75,8 @@ class ArmadaClient(object):
                    manifest_ref=None,
                    values=None,
                    set=None,
-                   query=None):
+                   query=None,
+                   timeout=None):
         """Call the Armada API to apply a Manifest.
 
         If ``manifest`` is not None, then the request body will be a fully
@@ -92,6 +94,7 @@ class ArmadaClient(object):
         :param values: list of local files containing values.yaml overrides
         :param set: list of single-value overrides
         :param query: explicit query string parameters
+        :param timeout: a tuple of connect, read timeout (x, y)
         """
         endpoint = self._set_endpoint('1.0', 'apply')
 
@@ -107,7 +110,8 @@ class ArmadaClient(object):
                 query=query,
                 headers={
                     'content-type': 'application/x-yaml'
-                })
+                },
+                timeout=timeout)
         elif manifest_ref:
             req_body = {
                 'hrefs': manifest_ref,
@@ -119,25 +123,27 @@ class ArmadaClient(object):
                 query=query,
                 headers={
                     'content-type': 'application/json'
-                })
+                },
+                timeout=timeout)
 
         self._check_response(resp)
 
         return resp.json()
 
-    def get_test_release(self, release=None, query=None):
+    def get_test_release(self, release=None, query=None, timeout=None):
 
         endpoint = self._set_endpoint('1.0', 'test/{}'.format(release))
-        resp = self.session.get(endpoint, query=query)
+        resp = self.session.get(endpoint, query=query, timeout=timeout)
 
         self._check_response(resp)
 
         return resp.json()
 
-    def post_test_manifest(self, manifest=None, query=None):
+    def post_test_manifest(self, manifest=None, query=None, timeout=None):
 
         endpoint = self._set_endpoint('1.0', 'tests')
-        resp = self.session.post(endpoint, body=manifest, query=query)
+        resp = self.session.post(endpoint, body=manifest, query=query,
+                                 timeout=timeout)
 
         self._check_response(resp)
 

armada/common/session.py

@@ -29,10 +29,14 @@ class ArmadaSession(object):
     :param int port: (optional) The service port appended if specified
     :param string token: Auth token
     :param string marker: (optional) external context marker
+    :param tuple timeout: (optional) a tuple of connect, read timeout values
+        to use as the default for invocations using this session. A single
+        value may also be supplied instead of a tuple to indicate only the
+        read timeout to use
     """
 
     def __init__(self, host, port=None, scheme='http', token=None,
-                 marker=None):
+                 marker=None, timeout=None):
 
         self._session = requests.Session()
         self._session.headers.update({
@@ -50,28 +54,33 @@ class ArmadaSession(object):
             self.base_url = "{}://{}/api/".format(
                 self.scheme, self.host)
 
+        self.default_timeout = ArmadaSession._calc_timeout_tuple((20, 3600),
+                                                                 timeout)
         self.token = token
         self.marker = marker
         self.logger = LOG
 
     # TODO Add keystone authentication to produce a token for this session
-    def get(self, endpoint, query=None, headers=None):
+    def get(self, endpoint, query=None, headers=None, timeout=None):
         """
         Send a GET request to armada.
 
         :param string endpoint: URL string following hostname and API prefix
         :param dict query: A dict of k, v pairs to add to the query string
         :param headers: Dictionary of HTTP headers to include in request
-
+        :param timeout: A single or tuple value for connect, read timeout.
+            A single value indicates the read timeout only
         :return: A requests.Response object
         """
         api_url = '{}{}'.format(self.base_url, endpoint)
         resp = self._session.get(
-            api_url, params=query, headers=headers, timeout=3600)
+            api_url, params=query, headers=headers,
+            timeout=self._timeout(timeout))
 
         return resp
 
-    def post(self, endpoint, query=None, body=None, data=None, headers=None):
+    def post(self, endpoint, query=None, body=None, data=None, headers=None,
+             timeout=None):
         """
         Send a POST request to armada. If both body and data are specified,
         body will will be used.
@@ -81,6 +90,8 @@ class ArmadaSession(object):
         :param string body: string to use as the request body.
         :param data: Something json.dumps(s) can serialize.
         :param headers: Dictionary of HTTP headers to include in request
+        :param timeout: A single or tuple value for connect, read timeout.
+            A single value indicates the read timeout only
         :return: A requests.Response object
         """
         api_url = '{}{}'.format(self.base_url, endpoint)
@@ -92,13 +103,52 @@ class ArmadaSession(object):
                                       params=query,
                                       data=body,
                                       headers=headers,
-                                      timeout=3600)
+                                      timeout=self._timeout(timeout))
         else:
             self.logger.debug("Sending POST with JSON body: \n%s" % str(data))
             resp = self._session.post(api_url,
                                       params=query,
                                       json=data,
                                       headers=headers,
-                                      timeout=3600)
+                                      timeout=self._timeout(timeout))
 
         return resp
+
+    def _timeout(self, timeout=None):
+        """Calculate the default timeouts for this session
+
+        :param timeout: A single or tuple value for connect, read timeout.
+            A single value indicates the read timeout only
+        :return: the tuple of the default timeouts used for this session
+        """
+        return ArmadaSession._calc_timeout_tuple(self.default_timeout, timeout)
+
+    @classmethod
+    def _calc_timeout_tuple(cls, def_timeout, timeout=None):
+        """Calculate the default timeouts for this session
+
+        :param def_timeout: The default timeout tuple to be used if no specific
+            timeout value is supplied
+        :param timeout: A single or tuple value for connect, read timeout.
+            A single value indicates the read timeout only
+        :return: the tuple of the timeouts calculated
+        """
+        connect_timeout, read_timeout = def_timeout
+
+        try:
+            if isinstance(timeout, tuple):
+                if all(isinstance(v, int)
+                        for v in timeout) and len(timeout) == 2:
+                    connect_timeout, read_timeout = timeout
+                else:
+                    raise ValueError("Tuple non-integer or wrong length")
+            elif isinstance(timeout, int):
+                read_timeout = timeout
+            elif timeout is not None:
+                raise ValueError("Non integer timeout value")
+        except ValueError:
+            LOG.warn("Timeout value must be a tuple of integers or a single"
+                     " integer. Proceeding with values of (%s, %s)",
+                     connect_timeout,
+                     read_timeout)
+        return (connect_timeout, read_timeout)

armada/tests/unit/common/test_session.py

@@ -0,0 +1,104 @@
+# Copyright 2017 The Armada Authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# import mock
+import testtools
+
+import responses
+
+from armada.common.session import ArmadaSession
+
+
+class SessionTestCase(testtools.TestCase):
+
+    def test_create_session(self):
+        """Tests setting up an Armada session"""
+        sess = ArmadaSession("testarmada")
+        assert sess.default_timeout == (20, 3600)
+
+        sess = ArmadaSession("testarmada", timeout=60)
+        assert sess.default_timeout == (20, 60)
+
+        sess = ArmadaSession("testarmada", timeout=(300, 300))
+        assert sess.default_timeout == (300, 300)
+
+        sess = ArmadaSession("testarmada", timeout=("cheese", "chicken"))
+        assert sess.default_timeout == (20, 3600)
+
+        sess = ArmadaSession("testarmada", timeout=(30, 60, 90))
+        assert sess.default_timeout == (20, 3600)
+
+        sess = ArmadaSession("testarmada", timeout="cranium")
+        assert sess.default_timeout == (20, 3600)
+
+    get_responses_inp = {
+        'method': 'GET',
+        'url': 'http://testarmada/api/bogus',
+        'body': 'got it',
+        'status': 200,
+        'content_type': 'text/plain'
+    }
+
+    @responses.activate
+    def test_get(self):
+        """Tests the get method"""
+        responses.add(**SessionTestCase.get_responses_inp)
+        sess = ArmadaSession("testarmada")
+        result = sess.get('bogus')
+        assert result.status_code == 200
+        return True
+
+    @responses.activate
+    def test_get_with_timeout(self):
+        """Tests the get method"""
+        responses.add(**SessionTestCase.get_responses_inp)
+        sess = ArmadaSession("testarmada")
+        result = sess.get('bogus', timeout=(60, 60))
+        assert result.status_code == 200
+        return True
+
+    post_responses_inp = {
+        'method': 'POST',
+        'url': 'http://testarmada/api/bogus',
+        'body': 'got it',
+        'status': 200,
+        'content_type': 'text/plain'
+    }
+
+    @responses.activate
+    def test_post(self):
+        """Tests the post method"""
+        responses.add(**SessionTestCase.post_responses_inp)
+        sess = ArmadaSession("testarmada")
+        result = sess.post('bogus')
+        assert result.status_code == 200
+        return True
+
+    @responses.activate
+    def test_post_with_timeout(self):
+        """Tests the post method"""
+        responses.add(**SessionTestCase.post_responses_inp)
+        sess = ArmadaSession("testarmada")
+        result = sess.post('bogus', timeout=(60, 60))
+        assert result.status_code == 200
+        return True
+
+    def test_timeout(self):
+        """Tests the _timeout method"""
+        sess = ArmadaSession("testarmada")
+        resp = sess._timeout((60, 70))
+        assert resp == (60, 70)
+
+        resp = sess._timeout()
+        assert resp == (20, 3600)

test-requirements.txt

@@ -14,3 +14,4 @@ bandit
 pytest==3.2.1
 pytest-mock
 pytest-cov
+responses>=0.8.1