From 7a2ba22ab12a3f1f180b6af4085972ba44853377 Mon Sep 17 00:00:00 2001 From: Bryan Strassner Date: Mon, 5 Mar 2018 17:24:15 -0600 Subject: [PATCH] [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 | 32 ++++--- armada/common/session.py | 64 ++++++++++++-- armada/tests/unit/common/test_session.py | 104 +++++++++++++++++++++++ test-requirements.txt | 1 + 4 files changed, 181 insertions(+), 20 deletions(-) create mode 100644 armada/tests/unit/common/test_session.py diff --git a/armada/common/client.py b/armada/common/client.py index 56b22bc3..a851db67 100644 --- a/armada/common/client.py +++ b/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) diff --git a/armada/common/session.py b/armada/common/session.py index 302e4fbb..502ea727 100644 --- a/armada/common/session.py +++ b/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) diff --git a/armada/tests/unit/common/test_session.py b/armada/tests/unit/common/test_session.py new file mode 100644 index 00000000..1bd8a5ef --- /dev/null +++ b/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) diff --git a/test-requirements.txt b/test-requirements.txt index 664503a0..02cc1a55 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -14,3 +14,4 @@ bandit pytest==3.2.1 pytest-mock pytest-cov +responses>=0.8.1