From e0c6ec0343a2a8056437b7605e90d75d05fb7b39 Mon Sep 17 00:00:00 2001 From: Dustin Falgout Date: Tue, 17 Nov 2015 10:40:03 -0600 Subject: [PATCH] incorporate feedback Signed-off-by: Dustin Falgout --- docker/utils/utils.py | 70 ++++++++++++++++------------- tests/integration/container_test.py | 18 ++------ tests/unit/api_test.py | 3 +- 3 files changed, 45 insertions(+), 46 deletions(-) diff --git a/docker/utils/utils.py b/docker/utils/utils.py index 46e3eb90..9c4bb477 100644 --- a/docker/utils/utils.py +++ b/docker/utils/utils.py @@ -236,7 +236,7 @@ def convert_port_bindings(port_bindings): for k, v in six.iteritems(port_bindings): key = str(k) if '/' not in key: - key = key + '/tcp' + key += '/tcp' if isinstance(v, list): result[key] = [_convert_port_binding(binding) for binding in v] else: @@ -434,7 +434,7 @@ def parse_bytes(s): s = 0 else: if s[-2:-1].isalpha() and s[-1].isalpha(): - if (s[-1] == "b" or s[-1] == "B"): + if s[-1] == "b" or s[-1] == "B": s = s[:-1] units = BYTE_UNITS suffix = s[-1].lower() @@ -467,17 +467,20 @@ def parse_bytes(s): return s -def host_config_type_error(param_value=None, expected=None): - return TypeError( - 'Invalid type for {0} param: expected {1} but found {2}'.format(param_value, expected, - type(param_value)) - ) +def host_config_type_error(param, param_value, expected): + error_msg = 'Invalid type for {0} param: expected {1} but found {2}' + return TypeError(error_msg.format(param, expected, type(param_value))) -def host_config_version_error(param=None, version=None): - return errors.InvalidVersion( - '{0} param is not supported in API versions < {1}'.format(param, version) - ) +def host_config_version_error(param, version, less_than=True): + operator = '<' if less_than else '>' + error_msg = '{0} param is not supported in API versions {1} {2}' + return errors.InvalidVersion(error_msg.format(param, operator, version)) + + +def host_config_value_error(param, param_value): + error_msg = 'Invalid value for {0} param: {1}' + return ValueError(error_msg.format(param, param_value)) def create_host_config(binds=None, port_bindings=None, lxc_conf=None, @@ -509,20 +512,21 @@ def create_host_config(binds=None, port_bindings=None, lxc_conf=None, if memswap_limit is not None: if isinstance(memswap_limit, six.string_types): memswap_limit = parse_bytes(memswap_limit) + host_config['MemorySwap'] = memswap_limit if mem_swappiness is not None: if version_lt(version, '1.20'): raise host_config_version_error('mem_swappiness', '1.20') if not isinstance(mem_swappiness, int): - raise host_config_type_error(mem_swappiness, 'int') + raise host_config_type_error( + 'mem_swappiness', mem_swappiness, 'int' + ) host_config['MemorySwappiness'] = mem_swappiness if pid_mode not in (None, 'host'): - raise errors.DockerException( - 'Invalid value for pid param: {0}'.format(pid_mode) - ) + raise host_config_value_error('pid_mode', pid_mode) elif pid_mode: host_config['PidMode'] = pid_mode @@ -533,10 +537,9 @@ def create_host_config(binds=None, port_bindings=None, lxc_conf=None, host_config['Privileged'] = privileged if oom_kill_disable: - if version_lt(version, '1.19'): - raise errors.InvalidVersion( - 'oom_kill_disable param not supported for API version < 1.19' - ) + if version_lt(version, '1.20'): + raise host_config_version_error('oom_kill_disable', '1.19') + host_config['OomKillDisable'] = oom_kill_disable if publish_all_ports: @@ -554,6 +557,11 @@ def create_host_config(binds=None, port_bindings=None, lxc_conf=None, host_config['NetworkMode'] = 'default' if restart_policy: + if not isinstance(restart_policy, dict): + raise host_config_type_error( + 'restart_policy', restart_policy, 'dict' + ) + host_config['RestartPolicy'] = restart_policy if cap_add: @@ -568,6 +576,7 @@ def create_host_config(binds=None, port_bindings=None, lxc_conf=None, if group_add: if version_lt(version, '1.20'): raise host_config_version_error('group_add', '1.20') + host_config['GroupAdd'] = [six.text_type(grp) for grp in group_add] if dns is not None: @@ -575,21 +584,21 @@ def create_host_config(binds=None, port_bindings=None, lxc_conf=None, if security_opt is not None: if not isinstance(security_opt, list): - raise host_config_type_error(security_opt, 'list') + raise host_config_type_error('security_opt', security_opt, 'list') + host_config['SecurityOpt'] = security_opt if volumes_from is not None: if isinstance(volumes_from, six.string_types): volumes_from = volumes_from.split(',') + host_config['VolumesFrom'] = volumes_from if binds is not None: host_config['Binds'] = convert_volume_binds(binds) if port_bindings is not None: - host_config['PortBindings'] = convert_port_bindings( - port_bindings - ) + host_config['PortBindings'] = convert_port_bindings(port_bindings) if extra_hosts is not None: if isinstance(extra_hosts, dict): @@ -604,9 +613,7 @@ def create_host_config(binds=None, port_bindings=None, lxc_conf=None, if isinstance(links, dict): links = six.iteritems(links) - formatted_links = [ - '{0}:{1}'.format(k, v) for k, v in sorted(links) - ] + formatted_links = ['{0}:{1}'.format(k, v) for k, v in sorted(links)] host_config['Links'] = formatted_links @@ -624,7 +631,7 @@ def create_host_config(binds=None, port_bindings=None, lxc_conf=None, if ulimits is not None: if not isinstance(ulimits, list): - raise host_config_type_error(ulimits, 'list') + raise host_config_type_error('ulimits', ulimits, 'list') host_config['Ulimits'] = [] for l in ulimits: if not isinstance(l, Ulimit): @@ -634,13 +641,16 @@ def create_host_config(binds=None, port_bindings=None, lxc_conf=None, if log_config is not None: if not isinstance(log_config, LogConfig): if not isinstance(log_config, dict): - raise host_config_type_error(log_config, 'LogConfig') + raise host_config_type_error( + 'log_config', log_config, 'LogConfig' + ) log_config = LogConfig(**log_config) + host_config['LogConfig'] = log_config if cpu_quota: if not isinstance(cpu_quota, int): - raise host_config_type_error(cpu_quota, 'int') + raise host_config_type_error('cpu_quota', cpu_quota, 'int') if version_lt(version, '1.19'): raise host_config_version_error('cpu_quota', '1.19') @@ -648,7 +658,7 @@ def create_host_config(binds=None, port_bindings=None, lxc_conf=None, if cpu_period: if not isinstance(cpu_period, int): - raise host_config_type_error(cpu_period, 'int') + raise host_config_type_error('cpu_period', cpu_period, 'int') if version_lt(version, '1.19'): raise host_config_version_error('cpu_period', '1.19') diff --git a/tests/integration/container_test.py b/tests/integration/container_test.py index 76a44189..79840a1b 100644 --- a/tests/integration/container_test.py +++ b/tests/integration/container_test.py @@ -365,21 +365,11 @@ class CreateContainerTest(helpers.BaseTestCase): self.assertIn('MemorySwappiness', host_config) def test_create_host_config_exception_raising(self): - with self.assertRaises(TypeError) as exc: - ctnr1 = self.client.create_container( - BUSYBOX, 'true', - host_config=self.client.create_host_config(mem_swappiness='40') - ) - self.assertIn('Invalid type for', exc.exception.response.text) - self.client.remove_container(ctnr1.get('Id', ctnr1), force=True) + self.assertRaises(TypeError, + self.client.create_host_config, mem_swappiness='40') - with self.assertRaises(docker.errors.InvalidVersion) as exc: - ctnr2 = self.client.create_container( - BUSYBOX, 'true', - host_config=self.client.create_host_config(version='1.18', mem_swappiness=40) - ) - self.assertIn('param is not supported in', exc.exception.response.text) - self.client.remove_container(ctnr2.get('Id', ctnr2), force=True) + self.assertRaises(ValueError, + self.client.create_host_config, pid_mode='40') class VolumeBindTest(helpers.BaseTestCase): diff --git a/tests/unit/api_test.py b/tests/unit/api_test.py index 62d64e8a..23fd1913 100644 --- a/tests/unit/api_test.py +++ b/tests/unit/api_test.py @@ -314,8 +314,7 @@ class DockerApiTest(DockerClientTest): self.assertIn('SecurityOpt', result) self.assertEqual(result['SecurityOpt'], security_opt) self.assertRaises( - docker.errors.DockerException, self.client.create_host_config, - security_opt='wrong' + TypeError, self.client.create_host_config, security_opt='wrong' )