From d2edaab531d0b452802851b077f9a0110bfa4b69 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Fri, 11 Jan 2013 21:56:24 -0800 Subject: [PATCH] Allow request timeout to be specified. Add a new cli argument (--timeout) which is by default 600 seconds which will be set in the requests library so that timeouts can occur correctly. Change-Id: I845c55dfb6f6b8345663ccdb5b150a2655f20026 --- keystoneclient/client.py | 11 +++++++++++ keystoneclient/shell.py | 31 ++++++++++++++++++++++++++++--- tests/test_shell.py | 40 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/keystoneclient/client.py b/keystoneclient/client.py index 6f2d1a452..c46dec604 100644 --- a/keystoneclient/client.py +++ b/keystoneclient/client.py @@ -60,6 +60,11 @@ class HTTPClient(object): cert=None, insecure=False, original_ip=None, debug=False, auth_ref=None, use_keyring=False, force_new_token=False, stale_duration=None): + """Construct a new http client + + @param: timeout the request libary timeout in seconds (default None) + + """ self.version = 'v2.0' # set baseline defaults self.username = None @@ -69,6 +74,10 @@ class HTTPClient(object): self.token = None self.auth_token = None self.management_url = None + if timeout is not None: + self.timeout = float(timeout) + else: + self.timeout = None # if loading from a dictionary passed in via auth_ref, # load values from AccessInfo parsing that dictionary self.auth_ref = access.AccessInfo(**auth_ref) if auth_ref else None @@ -320,6 +329,8 @@ class HTTPClient(object): del request_kwargs['body'] if self.cert: request_kwargs['cert'] = self.cert + if self.timeout is not None: + request_kwargs.setdefault('timeout', self.timeout) self.http_log_req((url, method,), request_kwargs) resp = requests.request( diff --git a/keystoneclient/shell.py b/keystoneclient/shell.py index ae8870f41..7d526194e 100644 --- a/keystoneclient/shell.py +++ b/keystoneclient/shell.py @@ -33,6 +33,20 @@ from keystoneclient.generic import shell as shell_generic from keystoneclient.contrib.bootstrap import shell as shell_bootstrap +def positive_non_zero_float(argument_value): + if argument_value is None: + return None + try: + value = float(argument_value) + except ValueError: + msg = "%s must be a float" % argument_value + raise argparse.ArgumentTypeError(msg) + if value <= 0: + msg = "%s must be greater than 0" % argument_value + raise argparse.ArgumentTypeError(msg) + return value + + def env(*vars, **kwargs): """Search for the first defined of possibly many env vars @@ -49,8 +63,11 @@ def env(*vars, **kwargs): class OpenStackIdentityShell(object): + def __init__(self, parser_class=argparse.ArgumentParser): + self.parser_class = parser_class + def get_base_parser(self): - parser = argparse.ArgumentParser( + parser = self.parser_class( prog='keystone', description=__doc__.strip(), epilog='See "keystone help COMMAND" ' @@ -74,6 +91,12 @@ class OpenStackIdentityShell(object): action='store_true', help=argparse.SUPPRESS) + parser.add_argument('--timeout', + default=600, + type=positive_non_zero_float, + metavar='', + help="Set request timeout (in seconds)") + parser.add_argument('--os-username', metavar='', default=env('OS_USERNAME'), @@ -385,7 +408,8 @@ class OpenStackIdentityShell(object): key=args.os_key, cert=args.os_cert, insecure=args.insecure, - debug=args.debug) + debug=args.debug, + timeout=args.timeout) else: token = None if args.os_token and args.os_endpoint: @@ -407,7 +431,8 @@ class OpenStackIdentityShell(object): debug=args.debug, use_keyring=args.os_cache, force_new_token=args.force_new_token, - stale_duration=args.stale_duration) + stale_duration=args.stale_duration, + timeout=args.timeout) try: args.func(self.cs, args) diff --git a/tests/test_shell.py b/tests/test_shell.py index bd8fdf09d..695fe0209 100644 --- a/tests/test_shell.py +++ b/tests/test_shell.py @@ -1,4 +1,5 @@ -import os +import argparse +import json import mock import fixtures @@ -17,6 +18,11 @@ DEFAULT_TENANT_NAME = 'tenant_name' DEFAULT_AUTH_URL = 'http://127.0.0.1:5000/v2.0/' +class NoExitArgumentParser(argparse.ArgumentParser): + def error(self, message): + raise exceptions.CommandError(message) + + class ShellTest(utils.TestCase): FAKE_ENV = { @@ -27,6 +33,10 @@ class ShellTest(utils.TestCase): 'OS_AUTH_URL': DEFAULT_AUTH_URL, } + def _tolerant_shell(self, cmd): + t_shell = openstack_shell.OpenStackIdentityShell(NoExitArgumentParser) + t_shell.main(cmd.split()) + # Patch os.environ to avoid required auth info. def setUp(self): @@ -292,6 +302,34 @@ class ShellTest(utils.TestCase): shell('ec2-credentials-delete') assert do_shell_mock.called + def test_timeout_parse_invalid_type(self): + for f in ['foobar', 'xyz']: + cmd = '--timeout %s endpoint-create' % (f) + self.assertRaises(exceptions.CommandError, + self._tolerant_shell, cmd) + + def test_timeout_parse_invalid_number(self): + for f in [-1, 0]: + cmd = '--timeout %s endpoint-create' % (f) + self.assertRaises(exceptions.CommandError, + self._tolerant_shell, cmd) + + def test_do_timeout(self): + response_mock = mock.MagicMock() + response_mock.status_code = 200 + response_mock.text = json.dumps({ + 'endpoints': [], + }) + request_mock = mock.MagicMock(return_value=response_mock) + with mock.patch('requests.request', request_mock): + shell(('--timeout 2 --os-token=blah --os-endpoint=blah' + ' --os-auth-url=blah.com endpoint-list')) + request_mock.assert_called_with(mock.ANY, mock.ANY, + timeout=2, + headers=mock.ANY, + verify=mock.ANY, + config=mock.ANY) + def test_do_endpoints(self): do_shell_mock = mock.MagicMock() # grab the decorators for do_endpoint_create