Skip to content

Commit 35b30e6

Browse files
committed
Remove validation of supported log drivers
By having this hardcoded list of log drivers, it is a bottleneck to us supporting more log drivers. The daemon already validates if a log driver is valid or not, so rather than duplicating that validation, let's pass the log_driver along. This allows support for new/more log drivers as they become supported in docker without having to wait for both docker-py and docker-compose to support them. Keeping the current list of log driver types for backwards compatibility. Signed-off-by: Mazz Mosley <[email protected]>
1 parent 90538cf commit 35b30e6

File tree

3 files changed

+26
-27
lines changed

3 files changed

+26
-27
lines changed

docker/utils/types.py

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,21 +20,17 @@ def __init__(self, init):
2020

2121

2222
class LogConfig(DictType):
23-
types = LogConfigTypesEnum
2423

2524
def __init__(self, **kwargs):
26-
type_ = kwargs.get('type', kwargs.get('Type'))
27-
config = kwargs.get('config', kwargs.get('Config'))
28-
if type_ not in self.types._values:
29-
raise ValueError("LogConfig.type must be one of ({0})".format(
30-
', '.join(self.types._values)
31-
))
25+
log_driver_type = kwargs.get('type', kwargs.get('Type'))
26+
config = kwargs.get('config', kwargs.get('Config')) or {}
27+
3228
if config and not isinstance(config, dict):
3329
raise ValueError("LogConfig.config must be a dictionary")
3430

3531
super(LogConfig, self).__init__({
36-
'Type': type_,
37-
'Config': config or {}
32+
'Type': log_driver_type,
33+
'Config': config
3834
})
3935

4036
@property
@@ -43,10 +39,6 @@ def type(self):
4339

4440
@type.setter
4541
def type(self, value):
46-
if value not in self.types._values:
47-
raise ValueError("LogConfig.type must be one of {0}".format(
48-
', '.join(self.types._values)
49-
))
5042
self['Type'] = value
5143

5244
@property

tests/integration_test.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
from six.moves import socketserver
3535

3636
from .test import Cleanup
37+
from docker.errors import APIError
3738

3839
# FIXME: missing tests for
3940
# export; history; insert; port; push; tag; get; load; stats
@@ -265,6 +266,22 @@ def test_valid_log_driver_and_log_opt(self):
265266
self.assertEqual(container_log_config['Type'], log_config.type)
266267
self.assertEqual(container_log_config['Config'], log_config.config)
267268

269+
def test_invalid_log_driver_raises_exception(self):
270+
log_config = docker.utils.LogConfig(
271+
type='asdf-nope',
272+
config={}
273+
)
274+
275+
container = self.client.create_container(
276+
'busybox', ['true'],
277+
host_config=create_host_config(log_config=log_config)
278+
)
279+
280+
expected_msg = "logger: no log driver named 'asdf-nope' is registered"
281+
with self.assertRaisesRegexp(APIError, expected_msg):
282+
# raises an internal server error 500
283+
self.client.start(container)
284+
268285

269286
@unittest.skipIf(not EXEC_DRIVER_IS_NATIVE, 'Exec driver not native')
270287
class TestCreateContainerReadOnlyFs(BaseTestCase):

tests/utils_test.py

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -192,29 +192,19 @@ def test_ulimit_invalid_type(self):
192192
self.assertRaises(ValueError, lambda: Ulimit(name='hello', hard='456'))
193193

194194
def test_create_host_config_dict_logconfig(self):
195-
dct = {'type': LogConfig.types.SYSLOG, 'config': {'key1': 'val1'}}
196-
config = create_host_config(
197-
log_config=dct, version=DEFAULT_DOCKER_API_VERSION
198-
)
195+
dct = {'type': 'syslog', 'config': {'key1': 'val1'}}
196+
config = create_host_config(log_config=dct)
199197
self.assertIn('LogConfig', config)
200198
self.assertTrue(isinstance(config['LogConfig'], LogConfig))
201199
self.assertEqual(dct['type'], config['LogConfig'].type)
202200

203201
def test_create_host_config_obj_logconfig(self):
204-
obj = LogConfig(type=LogConfig.types.SYSLOG, config={'key1': 'val1'})
205-
config = create_host_config(
206-
log_config=obj, version=DEFAULT_DOCKER_API_VERSION
207-
)
202+
obj = LogConfig(type='syslog', config={'key1': 'val1'})
203+
config = create_host_config(log_config=obj)
208204
self.assertIn('LogConfig', config)
209205
self.assertTrue(isinstance(config['LogConfig'], LogConfig))
210206
self.assertEqual(obj, config['LogConfig'])
211207

212-
def test_logconfig_invalid_type(self):
213-
self.assertRaises(ValueError, lambda: LogConfig(type='xxx', config={}))
214-
self.assertRaises(ValueError, lambda: LogConfig(
215-
type=LogConfig.types.JSON, config='helloworld'
216-
))
217-
218208
def test_resolve_repository_name(self):
219209
# docker hub library image
220210
self.assertEqual(

0 commit comments

Comments
 (0)