From cc3c455629fbbd334b4a92dec653c58ff5dad98b Mon Sep 17 00:00:00 2001 From: Maxime Petazzoni Date: Mon, 11 Nov 2013 14:02:49 -0800 Subject: [PATCH] Refactor timeout passing and standardize with keyword parameters Standardize all HTTP request calls to use keyword parameters for all but the URL. This makes the refactoring of including the timeout in these requests' parameters easier and more uniform-looking. Tweaks to the tests to comply with this new parameter passing scheme, in particular to the API calls assertions. Signed-off-by: Maxime Petazzoni --- docker/client.py | 114 +++++++++++++++++++++++------------------------ tests/test.py | 69 +++++++++++++--------------- 2 files changed, 85 insertions(+), 98 deletions(-) diff --git a/docker/client.py b/docker/client.py index 3fccd812..6ce3d66c 100644 --- a/docker/client.py +++ b/docker/client.py @@ -78,6 +78,21 @@ class Client(requests.Session): except Exception: pass + def _set_request_timeout(self, kwargs): + """Prepare the kwargs for an HTTP request by inserting the timeout + parameter, if not already present.""" + kwargs.setdefault('timeout', self._timeout) + return kwargs + + def _post(self, url, **kwargs): + return self.post(url, **self._set_request_timeout(kwargs)) + + def _get(self, url, **kwargs): + return self.get(url, **self._set_request_timeout(kwargs)) + + def _delete(self, url, **kwargs): + return self.delete(url, **self._set_request_timeout(kwargs)) + def _url(self, path): return '{0}/v{1}{2}'.format(self.base_url, self._version, path) @@ -148,8 +163,7 @@ class Client(requests.Session): if 'headers' not in kwargs: kwargs['headers'] = {} kwargs['headers']['Content-Type'] = 'application/json' - return self.post(url, json.dumps(data2), - timeout=self._timeout, **kwargs) + return self._post(url, data=json.dumps(data2), **kwargs) def attach_socket(self, container, params=None, ws=False): if ws: @@ -158,8 +172,8 @@ class Client(requests.Session): if isinstance(container, dict): container = container.get('Id') u = self._url("/containers/{0}/attach".format(container)) - res = self.post(u, None, params=self._attach_params(params), - stream=True, timeout=self._timeout) + res = self._post(u, params=self._attach_params(params), + stream=True) self._raise_for_status(res) # hijack the underlying socket from requests, icky # but for some reason requests.iter_contents and ilk @@ -220,9 +234,8 @@ class Client(requests.Session): } if context is not None: headers = {'Content-Type': 'application/tar'} - res = self._result(self.post(u, context, params=params, - headers=headers, stream=True, - timeout=self._timeout)) + res = self._result(self._post(u, data=context, params=params, + headers=headers, stream=True)) if context is not None: context.close() srch = r'Successfully built ([0-9a-f]+)' @@ -241,7 +254,7 @@ class Client(requests.Session): 'author': author } u = self._url("/commit") - return self._result(self._post_json(u, conf, params=params), json=True) + return self._result(self._post_json(u, data=conf, params=params), json=True) def containers(self, quiet=False, all=False, trunc=True, latest=False, since=None, before=None, limit=-1): @@ -253,8 +266,7 @@ class Client(requests.Session): 'before': before } u = self._url("/containers/ps") - res = self._result(self.get(u, params=params, timeout=self._timeout), - True) + res = self._result(self._get(u, params=params), True) if quiet: return [{'Id': x['Id']} for x in res] return res @@ -262,7 +274,7 @@ class Client(requests.Session): def copy(self, container, resource): res = self._post_json( self._url("/containers/{0}/copy".format(container)), - {"Resource": resource}, + data={"Resource": resource}, stream=True ) self._raise_for_status(res) @@ -285,40 +297,37 @@ class Client(requests.Session): params = { 'name': name } - res = self._post_json(u, config, params=params) + res = self._post_json(u, data=config, params=params) return self._result(res, True) def diff(self, container): if isinstance(container, dict): container = container.get('Id') - return self._result(self.get(self._url("/containers/{0}/changes". - format(container)), timeout=self._timeout), True) + return self._result(self._get(self._url("/containers/{0}/changes". + format(container))), True) def export(self, container): if isinstance(container, dict): container = container.get('Id') - res = self.get(self._url("/containers/{0}/export".format(container)), - stream=True, timeout=self._timeout) + res = self._get(self._url("/containers/{0}/export".format(container)), + stream=True) self._raise_for_status(res) return res.raw def history(self, image): - res = self.get(self._url("/images/{0}/history".format(image)), - timeout=self._timeout) + res = self._get(self._url("/images/{0}/history".format(image))) self._raise_for_status(res) return self._result(res) def images(self, name=None, quiet=False, all=False, viz=False): if viz: - return self._result(self.get(self._url("images/viz"), - timeout=self._timeout)) + return self._result(self._get(self._url("images/viz"))) params = { 'filter': name, 'only_ids': 1 if quiet else 0, 'all': 1 if all else 0, } - res = self._result(self.get(self._url("/images/json"), params=params, - timeout=self._timeout), + res = self._result(self._get(self._url("/images/json"), params=params), True) if quiet: return [x['Id'] for x in res] @@ -342,15 +351,12 @@ class Client(requests.Session): data = None if isinstance(src, six.string_types): params['fromSrc'] = src - return self._result(self.post(u, data, params=params, - timeout=self._timeout)) + return self._result(self._post(u, data=data, params=params)) - return self._result(self.post(u, src, params=params, - timeout=self._timeout)) + return self._result(self._post(u, data=src, params=params)) def info(self): - return self._result(self.get(self._url("/info"), - timeout=self._timeout), + return self._result(self._get(self._url("/info")), True) def insert(self, image, url, path): @@ -359,26 +365,23 @@ class Client(requests.Session): 'url': url, 'path': path } - return self._result(self.post(api_url, None, params=params, - timeout=self._timeout)) + return self._result(self._post(api_url, params=params)) def inspect_container(self, container): if isinstance(container, dict): container = container.get('Id') - return self._result(self.get(self._url("/containers/{0}/json".format(container)), - timeout=self._timeout), + return self._result(self._get(self._url("/containers/{0}/json".format(container))), True) def inspect_image(self, image_id): - return self._result(self.get(self._url("/images/{0}/json".format(image_id)), - timeout=self._timeout), + return self._result(self._get(self._url("/images/{0}/json".format(image_id))), True) def kill(self, container): if isinstance(container, dict): container = container.get('Id') url = self._url("/containers/{0}/kill".format(container)) - res = self.post(url, None, timeout=self._timeout) + res = self._post(url) self._raise_for_status(res) def login(self, username, password=None, email=None, registry=None): @@ -395,7 +398,7 @@ class Client(requests.Session): 'password': password, 'email': email } - res = self._result(self._post_json(url, req_data), True) + res = self._result(self._post_json(url, data=req_data), True) if res['Status'] == 'Login Succeeded': self._cfg['Configs'][registry] = req_data return res @@ -409,14 +412,12 @@ class Client(requests.Session): 'stderr': 1 } u = self._url("/containers/{0}/attach".format(container)) - return self._result(self.post(u, None, params=params, - timeout=self._timeout)) + return self._result(self._post(u, params=params)) def port(self, container, private_port): if isinstance(container, dict): container = container.get('Id') - res = self.get(self._url("/containers/{0}/json".format(container)), - timeout=self._timeout) + res = self._get(self._url("/containers/{0}/json".format(container))) self._raise_for_status(res) json_ = res.json() s_port = str(private_port) @@ -448,8 +449,7 @@ class Client(requests.Session): if authcfg: headers['X-Registry-Auth'] = auth.encode_header(authcfg) u = self._url("/images/create") - return self._result(self.post(u, params=params, headers=headers, - timeout=self._timeout)) + return self._result(self._post(u, params=params, headers=headers)) def push(self, repository): registry, repo_name = auth.resolve_repository_name(repository) @@ -463,19 +463,18 @@ class Client(requests.Session): # for this specific registry as we can have an anon push if authcfg: headers['X-Registry-Auth'] = auth.encode_header(authcfg) - return self._result(self._post_json(u, None, headers=headers)) - return self._result(self._post_json(u, authcfg)) + return self._result(self._post_json(u, headers=headers)) + return self._result(self._post_json(u, data=authcfg)) def remove_container(self, container, v=False): if isinstance(container, dict): container = container.get('Id') params = {'v': v} - res = self.delete(self._url("/containers/" + container), params=params, - timeout=self._timeout) + res = self._delete(self._url("/containers/" + container), params=params) self._raise_for_status(res) def remove_image(self, image): - res = self.delete(self._url("/images/" + image), timeout=self._timeout) + res = self._delete(self._url("/images/" + image)) self._raise_for_status(res) def restart(self, container, timeout=10): @@ -483,13 +482,12 @@ class Client(requests.Session): container = container.get('Id') params = {'t': timeout} url = self._url("/containers/{0}/restart".format(container)) - res = self.post(url, None, params=params, timeout=self._timeout) + res = self._post(url, params=params) self._raise_for_status(res) def search(self, term): - return self._result(self.get(self._url("/images/search"), - params={'term': term}, - timeout=self._timeout), + return self._result(self._get(self._url("/images/search"), + params={'term': term}), True) def start(self, container, binds=None, port_bindings=None, lxc_conf=None): @@ -515,7 +513,7 @@ class Client(requests.Session): start_config['PortBindings'] = port_bindings url = self._url("/containers/{0}/start".format(container)) - res = self._post_json(url, start_config) + res = self._post_json(url, data=start_config) self._raise_for_status(res) def stop(self, container, timeout=10): @@ -523,7 +521,7 @@ class Client(requests.Session): container = container.get('Id') params = {'t': timeout} url = self._url("/containers/{0}/stop".format(container)) - res = self.post(url, None, params=params, timeout=self._timeout) + res = self._post(url, params=params) self._raise_for_status(res) def tag(self, image, repository, tag=None, force=False): @@ -533,24 +531,22 @@ class Client(requests.Session): 'force': 1 if force else 0 } url = self._url("/images/{0}/tag".format(image)) - res = self.post(url, None, params=params, timeout=self._timeout) + res = self._post(url, params=params) self._raise_for_status(res) return res.status_code == 201 def top(self, container): u = self._url("/containers/{0}/top".format(container)) - return self._result(self.get(u, timeout=self._timeout), True) + return self._result(self._get(u), True) def version(self): - return self._result(self.get(self._url("/version"), - timeout=self._timeout), - True) + return self._result(self._get(self._url("/version")), True) def wait(self, container): if isinstance(container, dict): container = container.get('Id') url = self._url("/containers/{0}/wait".format(container)) - res = self.post(url, None, timeout=None) + res = self._post(url, timeout=None) self._raise_for_status(res) json_ = res.json() if 'StatusCode' in json_: diff --git a/tests/test.py b/tests/test.py index 02caf83d..f8f9baad 100644 --- a/tests/test.py +++ b/tests/test.py @@ -156,16 +156,16 @@ class DockerClientTest(unittest.TestCase): except Exception as e: self.fail('Command should not raise exception: {0}'.format(e)) - args = fake_request.call_args - self.assertEqual(args[0][0], + args, kwargs = fake_request.call_args + self.assertEqual(args[0], 'unix://var/run/docker.sock/v1.4/containers/create') - self.assertEqual(json.loads(args[0][1]), + self.assertEqual(json.loads(kwargs['data']), json.loads(''' {"Tty": false, "Image": "busybox", "Cmd": ["true"], "AttachStdin": false, "Memory": 0, "AttachStderr": true, "Privileged": false, "AttachStdout": true, "OpenStdin": false}''')) - self.assertEqual(args[1]['headers'], + self.assertEqual(kwargs['headers'], {'Content-Type': 'application/json'}) def test_create_container_with_binds(self): @@ -178,17 +178,17 @@ class DockerClientTest(unittest.TestCase): except Exception as e: self.fail('Command should not raise exception: {0}'.format(e)) - args = fake_request.call_args - self.assertEqual(args[0][0], + args, kwargs = fake_request.call_args + self.assertEqual(args[0], 'unix://var/run/docker.sock/v1.4/containers/create') - self.assertEqual(json.loads(args[0][1]), + self.assertEqual(json.loads(kwargs['data']), json.loads(''' {"Tty": false, "Image": "busybox", "Cmd": ["ls", "/mnt"], "AttachStdin": false, "Volumes": {"/mnt": {}}, "Memory": 0, "AttachStderr": true, "Privileged": false, "AttachStdout": true, "OpenStdin": false}''')) - self.assertEqual(args[1]['headers'], + self.assertEqual(kwargs['headers'], {'Content-Type': 'application/json'}) def test_create_container_privileged(self): @@ -197,16 +197,16 @@ class DockerClientTest(unittest.TestCase): except Exception as e: self.fail('Command should not raise exception: {0}'.format(e)) - args = fake_request.call_args - self.assertEqual(args[0][0], + args, kwargs = fake_request.call_args + self.assertEqual(args[0], 'unix://var/run/docker.sock/v1.4/containers/create') - self.assertEqual(json.loads(args[0][1]), + self.assertEqual(json.loads(kwargs['data']), json.loads(''' {"Tty": false, "Image": "busybox", "Cmd": ["true"], "AttachStdin": false, "Memory": 0, "AttachStderr": true, "Privileged": true, "AttachStdout": true, "OpenStdin": false}''')) - self.assertEqual(args[1]['headers'], + self.assertEqual(kwargs['headers'], {'Content-Type': 'application/json'}) def test_create_named_container(self): @@ -216,18 +216,18 @@ class DockerClientTest(unittest.TestCase): except Exception as e: self.fail('Command should not raise exception: {0}'.format(e)) - args = fake_request.call_args - self.assertEqual(args[0][0], + args, kwargs = fake_request.call_args + self.assertEqual(args[0], 'unix://var/run/docker.sock/v1.4/containers/create') - self.assertEqual(json.loads(args[0][1]), + self.assertEqual(json.loads(kwargs['data']), json.loads(''' {"Tty": false, "Image": "busybox", "Cmd": ["true"], "AttachStdin": false, "Memory": 0, "AttachStderr": true, "Privileged": false, "AttachStdout": true, "OpenStdin": false}''')) - self.assertEqual(args[1]['headers'], + self.assertEqual(kwargs['headers'], {'Content-Type': 'application/json'}) - self.assertEqual(args[1]['params'], {'name': 'marisa-kirisame'}) + self.assertEqual(kwargs['params'], {'name': 'marisa-kirisame'}) def test_start_container(self): try: @@ -237,7 +237,7 @@ class DockerClientTest(unittest.TestCase): fake_request.assert_called_with( 'unix://var/run/docker.sock/v1.4/containers/3cc2351ab11b/start', - '{}', + data='{}', headers={'Content-Type': 'application/json'}, timeout=docker.client.DEFAULT_TIMEOUT_SECONDS ) @@ -250,14 +250,14 @@ class DockerClientTest(unittest.TestCase): ) except Exception as e: self.fail('Command should not raise exception: {0}'.format(e)) - args = fake_request.call_args - self.assertEqual(args[0][0], 'unix://var/run/docker.sock/v1.4/' + args, kwargs = fake_request.call_args + self.assertEqual(args[0], 'unix://var/run/docker.sock/v1.4/' 'containers/3cc2351ab11b/start') self.assertEqual( - json.loads(args[0][1]), + json.loads(kwargs['data']), {"LxcConf": [{"Value": "lxc.conf.value", "Key": "lxc.conf.k"}]} ) - self.assertEqual(args[1]['headers'], + self.assertEqual(kwargs['headers'], {'Content-Type': 'application/json'}) def test_start_container_with_lxc_conf_compat(self): @@ -268,14 +268,14 @@ class DockerClientTest(unittest.TestCase): ) except Exception as e: self.fail('Command should not raise exception: {0}'.format(e)) - args = fake_request.call_args - self.assertEqual(args[0][0], 'unix://var/run/docker.sock/v1.4/' + args, kwargs = fake_request.call_args + self.assertEqual(args[0], 'unix://var/run/docker.sock/v1.4/' 'containers/3cc2351ab11b/start') self.assertEqual( - json.loads(args[0][1]), + json.loads(kwargs['data']), {"LxcConf": [{"Value": "lxc.conf.value", "Key": "lxc.conf.k"}]} ) - self.assertEqual(args[1]['headers'], + self.assertEqual(kwargs['headers'], {'Content-Type': 'application/json'}) def test_start_container_with_binds(self): @@ -289,7 +289,7 @@ class DockerClientTest(unittest.TestCase): fake_request.assert_called_with( 'unix://var/run/docker.sock/v1.4/containers/3cc2351ab11b/start', - '{"Binds": ["/tmp:/mnt"]}', + data='{"Binds": ["/tmp:/mnt"]}', headers={'Content-Type': 'application/json'}, timeout=docker.client.DEFAULT_TIMEOUT_SECONDS ) @@ -301,7 +301,8 @@ class DockerClientTest(unittest.TestCase): self.fail('Command should not raise exception: {0}'.format(e)) fake_request.assert_called_with( 'unix://var/run/docker.sock/v1.4/containers/3cc2351ab11b/start', - '{}', headers={'Content-Type': 'application/json'}, + data='{}', + headers={'Content-Type': 'application/json'}, timeout=docker.client.DEFAULT_TIMEOUT_SECONDS ) @@ -313,7 +314,6 @@ class DockerClientTest(unittest.TestCase): fake_request.assert_called_with( 'unix://var/run/docker.sock/v1.4/containers/3cc2351ab11b/wait', - None, timeout=None ) @@ -325,7 +325,6 @@ class DockerClientTest(unittest.TestCase): fake_request.assert_called_with( 'unix://var/run/docker.sock/v1.4/containers/3cc2351ab11b/wait', - None, timeout=None ) @@ -337,7 +336,6 @@ class DockerClientTest(unittest.TestCase): fake_request.assert_called_with( 'unix://var/run/docker.sock/v1.4/containers/3cc2351ab11b/attach', - None, params={'logs': 1, 'stderr': 1, 'stdout': 1}, timeout=docker.client.DEFAULT_TIMEOUT_SECONDS ) @@ -350,7 +348,6 @@ class DockerClientTest(unittest.TestCase): fake_request.assert_called_with( 'unix://var/run/docker.sock/v1.4/containers/3cc2351ab11b/attach', - None, params={'logs': 1, 'stderr': 1, 'stdout': 1}, timeout=docker.client.DEFAULT_TIMEOUT_SECONDS ) @@ -383,7 +380,6 @@ class DockerClientTest(unittest.TestCase): fake_request.assert_called_with( 'unix://var/run/docker.sock/v1.4/containers/3cc2351ab11b/stop', - None, params={'t': 2}, timeout=docker.client.DEFAULT_TIMEOUT_SECONDS ) @@ -396,7 +392,6 @@ class DockerClientTest(unittest.TestCase): fake_request.assert_called_with( 'unix://var/run/docker.sock/v1.4/containers/3cc2351ab11b/stop', - None, params={'t': 2}, timeout=docker.client.DEFAULT_TIMEOUT_SECONDS ) @@ -409,7 +404,6 @@ class DockerClientTest(unittest.TestCase): fake_request.assert_called_with( 'unix://var/run/docker.sock/v1.4/containers/3cc2351ab11b/kill', - None, timeout=docker.client.DEFAULT_TIMEOUT_SECONDS ) @@ -421,7 +415,6 @@ class DockerClientTest(unittest.TestCase): fake_request.assert_called_with( 'unix://var/run/docker.sock/v1.4/containers/3cc2351ab11b/kill', - None, timeout=docker.client.DEFAULT_TIMEOUT_SECONDS ) @@ -433,7 +426,6 @@ class DockerClientTest(unittest.TestCase): fake_request.assert_called_with( 'unix://var/run/docker.sock/v1.4/containers/3cc2351ab11b/restart', - None, params={'t': 2}, timeout=docker.client.DEFAULT_TIMEOUT_SECONDS ) @@ -446,7 +438,6 @@ class DockerClientTest(unittest.TestCase): fake_request.assert_called_with( 'unix://var/run/docker.sock/v1.4/containers/3cc2351ab11b/restart', - None, params={'t': 2}, timeout=docker.client.DEFAULT_TIMEOUT_SECONDS ) @@ -500,7 +491,7 @@ class DockerClientTest(unittest.TestCase): fake_request.assert_called_with( 'unix://var/run/docker.sock/v1.4/commit', - '{}', + data='{}', headers={'Content-Type': 'application/json'}, params={ 'repo': None,