From e4873c451630181bb593ce703b8f0d57947de3a7 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 15 Apr 2013 18:14:52 +0200 Subject: [PATCH 01/17] config: Return convereted values and errors --- mopidy/config/__init__.py | 21 +++++++--------- mopidy/config/schemas.py | 37 +++++++++++++--------------- tests/config/schemas_test.py | 47 ++++++++++++++++-------------------- 3 files changed, 47 insertions(+), 58 deletions(-) diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 4aac8dec..9d33bcb6 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -90,8 +90,9 @@ def validate(raw_config, schemas, extensions=None): if errors: # TODO: raise error instead. #raise exceptions.ConfigError(errors) - for error in errors: - logger.error(error) + for section in errors: + for key, error in errors[section].items(): + logger.error('Config value %s/%s %s', section, key, error) sys.exit(1) return config @@ -101,17 +102,13 @@ def validate(raw_config, schemas, extensions=None): def _validate(raw_config, schemas): # Get validated config config = {} - errors = [] + errors = {} for schema in schemas: - try: - items = raw_config[schema.name].items() - config[schema.name] = schema.convert(items) - except KeyError: - errors.append('%s: section not found.' % schema.name) - except exceptions.ConfigError as error: - for key in error: - errors.append('%s/%s: %s' % (schema.name, key, error[key])) - # TODO: raise errors instead of return + values = raw_config.get(schema.name, {}) + result, error = schema.convert(values) + if error: + errors[schema.name] = error + config[schema.name] = result return config, errors diff --git a/mopidy/config/schemas.py b/mopidy/config/schemas.py index 9e117d67..72c0ee6d 100644 --- a/mopidy/config/schemas.py +++ b/mopidy/config/schemas.py @@ -71,15 +71,16 @@ class ConfigSchema(object): key, self._schema[key].format(value))) return '\n'.join(lines) - def convert(self, items): - """Validates the given ``items`` using the config schema and returns - clean values""" - errors = {} - values = {} + def convert(self, values): + """Validates the given ``values`` using the config schema. - for key, value in items: + Returns a tuple with cleaned values and errors.""" + errors = {} + result = {} + + for key, value in values.items(): try: - values[key] = self._schema[key].deserialize(value) + result[key] = self._schema[key].deserialize(value) except KeyError: # not in our schema errors[key] = 'unknown config key.' suggestion = _did_you_mean(key, self._schema.keys()) @@ -89,12 +90,10 @@ class ConfigSchema(object): errors[key] = str(e) for key in self._schema: - if key not in values and key not in errors: + if key not in result and key not in errors: errors[key] = 'config key not found.' - if errors: - raise exceptions.ConfigError(errors) - return values + return result, errors class ExtensionConfigSchema(ConfigSchema): @@ -106,6 +105,8 @@ class ExtensionConfigSchema(ConfigSchema): super(ExtensionConfigSchema, self).__init__(name) self['enabled'] = types.Boolean() + # TODO: override convert to gate on enabled=true? + class LogLevelConfigSchema(object): """Special cased schema for handling a config section with loglevels. @@ -126,17 +127,13 @@ class LogLevelConfigSchema(object): key, self._config_value.format(value))) return '\n'.join(lines) - def convert(self, items): + def convert(self, values): errors = {} - values = {} + result = {} - for key, value in items: + for key, value in values.items(): try: - if value.strip(): - values[key] = self._config_value.deserialize(value) + result[key] = self._config_value.deserialize(value) except ValueError as e: # deserialization failed errors[key] = str(e) - - if errors: - raise exceptions.ConfigError(errors) - return values + return result, errors diff --git a/tests/config/schemas_test.py b/tests/config/schemas_test.py index 6274f66c..80f06f39 100644 --- a/tests/config/schemas_test.py +++ b/tests/config/schemas_test.py @@ -36,54 +36,49 @@ class ConfigSchemaTest(unittest.TestCase): self.assertNotIn('unknown = rty', result) def test_convert(self): - self.schema.convert(self.values.items()) + self.schema.convert(self.values) def test_convert_with_missing_value(self): del self.values['foo'] - with self.assertRaises(exceptions.ConfigError) as cm: - self.schema.convert(self.values.items()) - - self.assertIn('not found', cm.exception['foo']) + result, errors = self.schema.convert(self.values) + self.assertIn('not found', errors['foo']) + self.assertItemsEqual(['bar', 'baz'], result.keys()) def test_convert_with_extra_value(self): self.values['extra'] = '123' - with self.assertRaises(exceptions.ConfigError) as cm: - self.schema.convert(self.values.items()) - - self.assertIn('unknown', cm.exception['extra']) + result, errors = self.schema.convert(self.values) + self.assertIn('unknown', errors['extra']) + self.assertItemsEqual(['foo', 'bar', 'baz'], result.keys()) def test_convert_with_deserialization_error(self): self.schema['foo'].deserialize.side_effect = ValueError('failure') - with self.assertRaises(exceptions.ConfigError) as cm: - self.schema.convert(self.values.items()) - - self.assertIn('failure', cm.exception['foo']) + result, errors = self.schema.convert(self.values) + self.assertIn('failure', errors['foo']) + self.assertItemsEqual(['bar', 'baz'], result.keys()) def test_convert_with_multiple_deserialization_errors(self): self.schema['foo'].deserialize.side_effect = ValueError('failure') self.schema['bar'].deserialize.side_effect = ValueError('other') - with self.assertRaises(exceptions.ConfigError) as cm: - self.schema.convert(self.values.items()) - - self.assertIn('failure', cm.exception['foo']) - self.assertIn('other', cm.exception['bar']) + result, errors = self.schema.convert(self.values) + self.assertIn('failure', errors['foo']) + self.assertIn('other', errors['bar']) + self.assertItemsEqual(['baz'], result.keys()) def test_convert_deserialization_unknown_and_missing_errors(self): self.values['extra'] = '123' self.schema['bar'].deserialize.side_effect = ValueError('failure') del self.values['baz'] - with self.assertRaises(exceptions.ConfigError) as cm: - self.schema.convert(self.values.items()) - - self.assertIn('unknown', cm.exception['extra']) - self.assertNotIn('foo', cm.exception) - self.assertIn('failure', cm.exception['bar']) - self.assertIn('not found', cm.exception['baz']) + result, errors = self.schema.convert(self.values) + self.assertIn('unknown', errors['extra']) + self.assertNotIn('foo', errors) + self.assertIn('failure', errors['bar']) + self.assertIn('not found', errors['baz']) + self.assertItemsEqual(['foo'], result.keys()) class ExtensionConfigSchemaTest(unittest.TestCase): @@ -95,7 +90,7 @@ class ExtensionConfigSchemaTest(unittest.TestCase): class LogLevelConfigSchemaTest(unittest.TestCase): def test_conversion(self): schema = schemas.LogLevelConfigSchema('test') - result = schema.convert([('foo.bar', 'DEBUG'), ('baz', 'INFO')]) + result, errors = schema.convert({'foo.bar': 'DEBUG', 'baz': 'INFO'}) self.assertEqual(logging.DEBUG, result['foo.bar']) self.assertEqual(logging.INFO, result['baz']) From f5cd806dc5340326d55769a1227222483fe79cf2 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 15 Apr 2013 19:50:51 +0200 Subject: [PATCH 02/17] config: Rename convert to deserialize --- mopidy/config/__init__.py | 5 +-- mopidy/config/schemas.py | 63 ++++++++++++++++++++---------------- tests/config/config_test.py | 20 ++++++------ tests/config/schemas_test.py | 27 ++++++++-------- 4 files changed, 62 insertions(+), 53 deletions(-) diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 9d33bcb6..85e0fa61 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -105,10 +105,11 @@ def _validate(raw_config, schemas): errors = {} for schema in schemas: values = raw_config.get(schema.name, {}) - result, error = schema.convert(values) + result, error = schema.deserialize(values) if error: errors[schema.name] = error - config[schema.name] = result + if result: + config[schema.name] = result return config, errors diff --git a/mopidy/config/schemas.py b/mopidy/config/schemas.py index 72c0ee6d..90d966d3 100644 --- a/mopidy/config/schemas.py +++ b/mopidy/config/schemas.py @@ -40,9 +40,11 @@ class ConfigSchema(object): """Logical group of config values that correspond to a config section. Schemas are set up by assigning config keys with config values to - instances. Once setup :meth:`convert` can be called with a list of - ``(key, value)`` tuples to process. For convienience we also support - :meth:`format` method that can used for printing out the converted values. + instances. Once setup :meth:`deserialize` can be called with a dict of + values to process. For convienience we also support :meth:`format` method + that can used for converting the values to a dict that can be printed and + :meth:`serialize` for converting the values to a form suitable for + persistence. """ # TODO: Use collections.OrderedDict once 2.6 support is gone (#344) def __init__(self, name): @@ -58,20 +60,7 @@ class ConfigSchema(object): def __getitem__(self, key): return self._schema[key] - def format(self, values): - """Returns the schema as a config section with the given ``values`` - filled in""" - # TODO: should the output be encoded utf-8 since we use that in - # serialize for strings? - lines = ['[%s]' % self.name] - for key in self._order: - value = values.get(key) - if value is not None: - lines.append('%s = %s' % ( - key, self._schema[key].format(value))) - return '\n'.join(lines) - - def convert(self, values): + def deserialize(self, values): """Validates the given ``values`` using the config schema. Returns a tuple with cleaned values and errors.""" @@ -95,6 +84,23 @@ class ConfigSchema(object): return result, errors + def serialize(self, values): + pass + + def format(self, values): + """Returns the schema as a config section with the given ``values`` + filled in""" + # TODO: should the output be encoded utf-8 since we use that in + # serialize for strings? + lines = ['[%s]' % self.name] + for key in self._order: + value = values.get(key) + if value is not None: + lines.append('%s = %s' % ( + key, self._schema[key].format(value))) + return '\n'.join(lines) + + class ExtensionConfigSchema(ConfigSchema): """Sub-classed :class:`ConfigSchema` for use in extensions. @@ -105,7 +111,7 @@ class ExtensionConfigSchema(ConfigSchema): super(ExtensionConfigSchema, self).__init__(name) self['enabled'] = types.Boolean() - # TODO: override convert to gate on enabled=true? + # TODO: override serialize to gate on enabled=true? class LogLevelConfigSchema(object): @@ -119,15 +125,7 @@ class LogLevelConfigSchema(object): self.name = name self._config_value = types.LogLevel() - def format(self, values): - lines = ['[%s]' % self.name] - for key, value in sorted(values.items()): - if value is not None: - lines.append('%s = %s' % ( - key, self._config_value.format(value))) - return '\n'.join(lines) - - def convert(self, values): + def deserialize(self, values): errors = {} result = {} @@ -137,3 +135,14 @@ class LogLevelConfigSchema(object): except ValueError as e: # deserialization failed errors[key] = str(e) return result, errors + + def serialize(self, values): + pass + + def format(self, values): + lines = ['[%s]' % self.name] + for key, value in sorted(values.items()): + if value is not None: + lines.append('%s = %s' % ( + key, self._config_value.format(value))) + return '\n'.join(lines) diff --git a/tests/config/config_test.py b/tests/config/config_test.py index c90486c7..b112de1f 100644 --- a/tests/config/config_test.py +++ b/tests/config/config_test.py @@ -53,38 +53,38 @@ class LoadConfigTest(unittest.TestCase): class ValidateTest(unittest.TestCase): def setUp(self): - self.schema = mock.Mock() - self.schema.name = 'foo' + self.schema = config.ConfigSchema('foo') + self.schema['bar'] = config.ConfigValue() def test_empty_config_no_schemas(self): conf, errors = config._validate({}, []) self.assertEqual({}, conf) - self.assertEqual([], errors) + self.assertEqual({}, errors) def test_config_no_schemas(self): raw_config = {'foo': {'bar': 'baz'}} conf, errors = config._validate(raw_config, []) self.assertEqual({}, conf) - self.assertEqual([], errors) + self.assertEqual({}, errors) def test_empty_config_single_schema(self): conf, errors = config._validate({}, [self.schema]) self.assertEqual({}, conf) - self.assertEqual(['foo: section not found.'], errors) + self.assertEqual({'foo': {'bar': 'config key not found.'}}, errors) def test_config_single_schema(self): raw_config = {'foo': {'bar': 'baz'}} - self.schema.convert.return_value = {'baz': 'bar'} conf, errors = config._validate(raw_config, [self.schema]) - self.assertEqual({'foo': {'baz': 'bar'}}, conf) - self.assertEqual([], errors) + self.assertEqual({'foo': {'bar': 'baz'}}, conf) + self.assertEqual({}, errors) def test_config_single_schema_config_error(self): raw_config = {'foo': {'bar': 'baz'}} - self.schema.convert.side_effect = exceptions.ConfigError({'bar': 'bad'}) + self.schema['bar'] = mock.Mock() + self.schema['bar'].deserialize.side_effect = ValueError('bad') conf, errors = config._validate(raw_config, [self.schema]) - self.assertEqual(['foo/bar: bad'], errors) self.assertEqual({}, conf) + self.assertEqual({'foo': {'bar': 'bad'}}, errors) # TODO: add more tests diff --git a/tests/config/schemas_test.py b/tests/config/schemas_test.py index 80f06f39..f8d908f7 100644 --- a/tests/config/schemas_test.py +++ b/tests/config/schemas_test.py @@ -35,45 +35,44 @@ class ConfigSchemaTest(unittest.TestCase): result = self.schema.format(self.values) self.assertNotIn('unknown = rty', result) - def test_convert(self): - self.schema.convert(self.values) + def test_deserialize(self): + self.schema.deserialize(self.values) - def test_convert_with_missing_value(self): + def test_deserialize_with_missing_value(self): del self.values['foo'] - result, errors = self.schema.convert(self.values) + result, errors = self.schema.deserialize(self.values) self.assertIn('not found', errors['foo']) self.assertItemsEqual(['bar', 'baz'], result.keys()) - def test_convert_with_extra_value(self): + def test_deserialize_with_extra_value(self): self.values['extra'] = '123' - result, errors = self.schema.convert(self.values) + result, errors = self.schema.deserialize(self.values) self.assertIn('unknown', errors['extra']) self.assertItemsEqual(['foo', 'bar', 'baz'], result.keys()) - def test_convert_with_deserialization_error(self): + def test_deserialize_with_deserialization_error(self): self.schema['foo'].deserialize.side_effect = ValueError('failure') - - result, errors = self.schema.convert(self.values) + result, errors = self.schema.deserialize(self.values) self.assertIn('failure', errors['foo']) self.assertItemsEqual(['bar', 'baz'], result.keys()) - def test_convert_with_multiple_deserialization_errors(self): + def test_deserialize_with_multiple_deserialization_errors(self): self.schema['foo'].deserialize.side_effect = ValueError('failure') self.schema['bar'].deserialize.side_effect = ValueError('other') - result, errors = self.schema.convert(self.values) + result, errors = self.schema.deserialize(self.values) self.assertIn('failure', errors['foo']) self.assertIn('other', errors['bar']) self.assertItemsEqual(['baz'], result.keys()) - def test_convert_deserialization_unknown_and_missing_errors(self): + def test_deserialize_deserialization_unknown_and_missing_errors(self): self.values['extra'] = '123' self.schema['bar'].deserialize.side_effect = ValueError('failure') del self.values['baz'] - result, errors = self.schema.convert(self.values) + result, errors = self.schema.deserialize(self.values) self.assertIn('unknown', errors['extra']) self.assertNotIn('foo', errors) self.assertIn('failure', errors['bar']) @@ -90,7 +89,7 @@ class ExtensionConfigSchemaTest(unittest.TestCase): class LogLevelConfigSchemaTest(unittest.TestCase): def test_conversion(self): schema = schemas.LogLevelConfigSchema('test') - result, errors = schema.convert({'foo.bar': 'DEBUG', 'baz': 'INFO'}) + result, errors = schema.deserialize({'foo.bar': 'DEBUG', 'baz': 'INFO'}) self.assertEqual(logging.DEBUG, result['foo.bar']) self.assertEqual(logging.INFO, result['baz']) From 51f89017fe5cc9934f1a68a9eff49bcc0d158f4f Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 15 Apr 2013 21:11:12 +0200 Subject: [PATCH 03/17] config: Fix handling of None in Secret --- mopidy/config/types.py | 19 ++++++++++++++++--- tests/config/types_test.py | 27 ++++++++++++++++++++++++--- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/mopidy/config/types.py b/mopidy/config/types.py index 6543b62e..c43da740 100644 --- a/mopidy/config/types.py +++ b/mopidy/config/types.py @@ -80,23 +80,36 @@ class String(ConfigValue): return value def serialize(self, value): + if value is None: + return b'' return encode(value) class Secret(ConfigValue): - """String value. + """Secret value. - Masked when being displayed, and is not decoded. + Should be used for passwords, auth tokens etc. Deserializing will not + convert to unicode. Will mask value when being displayed. """ def __init__(self, optional=False, choices=None): self._required = not optional def deserialize(self, value): + value = value.strip() validators.validate_required(value, self._required) + if not value: + return None + return value + + def serialize(self, value): + if value is None: + return b'' return value def format(self, value): - return '********' + if value is None: + return b'' + return b'********' class Integer(ConfigValue): diff --git a/tests/config/types_test.py b/tests/config/types_test.py index fe616b77..a5f2c9dc 100644 --- a/tests/config/types_test.py +++ b/tests/config/types_test.py @@ -88,6 +88,12 @@ class StringTest(unittest.TestCase): self.assertIsInstance(result, bytes) self.assertEqual(r'a\n\tb'.encode('utf-8'), result) + def test_serialize_none(self): + value = types.String() + result = value.serialize(None) + self.assertIsInstance(result, bytes) + self.assertEqual(b'', result) + class SecretTest(unittest.TestCase): def test_deserialize_passes_through(self): @@ -100,13 +106,28 @@ class SecretTest(unittest.TestCase): value = types.Secret() self.assertRaises(ValueError, value.deserialize, b'') - def test_serialize_conversion_to_string(self): + def test_deserialize_respects_optional(self): + value = types.Secret(optional=True) + self.assertIsNone(value.deserialize(b'')) + self.assertIsNone(value.deserialize(b' ')) + + def test_serialize_none(self): value = types.Secret() - self.assertIsInstance(value.serialize(object()), bytes) + result = value.serialize(None) + self.assertIsInstance(result, bytes) + self.assertEqual(b'', result) def test_format_masks_value(self): value = types.Secret() - self.assertEqual('********', value.format('s3cret')) + result = value.format('s3cret') + self.assertIsInstance(result, bytes) + self.assertEqual(b'********', result) + + def test_format_none(self): + value = types.Secret() + result = value.format(None) + self.assertIsInstance(result, bytes) + self.assertEqual(b'', result) class IntegerTest(unittest.TestCase): From 611af060f6f25cad006f7cd27522ce52378887ad Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 15 Apr 2013 22:26:45 +0200 Subject: [PATCH 04/17] config: Add core_defaults to config --- mopidy/config/__init__.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 85e0fa61..20c2315e 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -12,6 +12,8 @@ from mopidy.utils import path logger = logging.getLogger('mopdiy.config') +_config_dir = os.path.dirname(__file__) + _logging_schema = ConfigSchema('logging') _logging_schema['console_format'] = String() _logging_schema['debug_format'] = String() @@ -32,8 +34,12 @@ _proxy_schema['password'] = Secret(optional=True) # NOTE: if multiple outputs ever comes something like LogLevelConfigSchema #_outputs_schema = config.AudioOutputConfigSchema() +#: Config schemas used by mopidy itself. core_schemas = [_logging_schema, _loglevels_schema, _audio_schema, _proxy_schema] +#: Config default used by mopidy itself. +core_defaults = io.open(os.path.join(_config_dir, 'default.conf'), 'rb').read() + def read(config_file): """Helper to load config defaults in same way across core and extensions""" From 86cad84f7d2a8be37aa2077ba83196a140827302 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 15 Apr 2013 22:41:07 +0200 Subject: [PATCH 05/17] main: Refactor to use underlying _load and _validate functions from config --- mopidy/__main__.py | 74 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 59 insertions(+), 15 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 163f67ae..03da85b9 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -42,33 +42,46 @@ def main(): config_files = options.config.split(':') config_overrides = options.overrides - extensions = [] # Make sure it is defined before the finally block + enabled_extensions = [] # Make sure it is defined before the finally block # TODO: figure out a way to make the boilerplate in this file reusable in # scanner and other places we need it. try: - create_file_structures() - # TODO: run raw logging config trough escape code etc, or just validate? - logging_config = config_lib.load(config_files, config_overrides) + # Initial config without extensions to bootstrap logging. + logging_config, _ = get_config(config_files, [], config_overrides) + + # TODO: setup_logging needs defaults in-case config values are None log.setup_logging( logging_config, options.verbosity_level, options.save_debug_log) - extensions = ext.load_extensions() - raw_config = config_lib.load(config_files, config_overrides, extensions) - extensions = ext.filter_enabled_extensions(raw_config, extensions) - config = config_lib.validate( - raw_config, config_lib.core_schemas, extensions) - log.setup_log_levels(config) - check_old_locations() + + all_extensions = ext.load_extensions() # TODO: wrap config in RO proxy. + config, config_errors = get_config( + config_files, all_extensions, config_overrides) + + # Filter out disabled extensions and remove any config errors for them. + # TODO: extension.should_install(config) + for extension in all_extensions: + if config[extension.ext_name]['enabled']: + enabled_extensions.append(extension) + elif extension.ext_name in config_errors: + del config_errors[extension.ext_name] + + log_extension_info(all_extensions, enabled_extensions) + check_config_errors(config_errors) + + log.setup_log_levels(config) + create_file_structures() + check_old_locations() # Anything that wants to exit after this point must use # mopidy.utils.process.exit_process as actors have been started. audio = setup_audio(config) - backends = setup_backends(config, extensions, audio) + backends = setup_backends(config, enabled_extensions, audio) core = setup_core(audio, backends) - setup_frontends(config, extensions, core) + setup_frontends(config, enabled_extensions, core) loop.run() except KeyboardInterrupt: logger.info('Interrupted. Exiting...') @@ -76,13 +89,44 @@ def main(): logger.exception(ex) finally: loop.quit() - stop_frontends(extensions) + stop_frontends(enabled_extensions) stop_core() - stop_backends(extensions) + stop_backends(enabled_extensions) stop_audio() process.stop_remaining_actors() +def get_config(files, extensions, overrides): + # Helper to get configs, as our config system should not need to know about + # extensions. + defaults = [config_lib.core_defaults] + defaults.extend(e.get_default_config() for e in extensions) + raw_config = config_lib._load(files, defaults, overrides) + + schemas = config_lib.core_schemas[:] + schemas.extend(e.get_config_schema() for e in extensions) + return config_lib._validate(raw_config, schemas) + + +def log_extension_info(all_extensions, enabled_extensions): + # TODO: distinguish disabled vs blocked by env? + enabled_names = set(e.ext_name for e in enabled_extensions) + disabled_names = set(e.ext_name for e in all_extensions) - enabled_names + logging.info( + 'Enabled extensions: %s', ', '.join(enabled_names) or 'none') + logging.info( + 'Disabled extensions: %s', ', '.join(disabled_names) or 'none') + + +def check_config_errors(errors): + if not errors: + return + for section in errors: + for key, msg in errors[section].items(): + logger.error('Config value %s/%s %s', section, key, msg) + sys.exit(1) + + def check_config_override(option, opt, override): try: return config_lib.parse_override(override) From 7e154bdcbf76372a3510ab66aea13a33eaec2207 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 15 Apr 2013 22:46:47 +0200 Subject: [PATCH 06/17] ext: Remove filter_enabled_extensions --- mopidy/ext.py | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/mopidy/ext.py b/mopidy/ext.py index 9cea1bd5..9d30afd1 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -127,25 +127,3 @@ def load_extensions(): names = (e.ext_name for e in extensions) logging.debug('Discovered extensions: %s', ', '.join(names)) return extensions - - -def filter_enabled_extensions(raw_config, extensions): - boolean = config_lib.Boolean() - enabled_extensions = [] - enabled_names = [] - disabled_names = [] - - for extension in extensions: - # TODO: handle key and value errors. - enabled = raw_config[extension.ext_name]['enabled'] - if boolean.deserialize(enabled): - enabled_extensions.append(extension) - enabled_names.append(extension.ext_name) - else: - disabled_names.append(extension.ext_name) - - logging.info( - 'Enabled extensions: %s', ', '.join(enabled_names) or 'none') - logging.info( - 'Disabled extensions: %s', ', '.join(disabled_names) or 'none') - return enabled_extensions From 992b2931744082b22aa4abb8bd420852a63685da Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 15 Apr 2013 23:03:40 +0200 Subject: [PATCH 07/17] ext: Switch to validate_extension(extension, config) --- mopidy/__main__.py | 12 +++++----- mopidy/ext.py | 56 +++++++++++++++++++++------------------------- 2 files changed, 30 insertions(+), 38 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 1a134cc5..ca99238a 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -56,27 +56,24 @@ def main(): logging_config, options.verbosity_level, options.save_debug_log) installed_extensions = ext.load_extensions() - all_extensions = ext.validate_extensions(installed_extensions) # TODO: wrap config in RO proxy. config, config_errors = get_config( - config_files, all_extensions, config_overrides) + config_files, installed_extensions, config_overrides) # Filter out disabled extensions and remove any config errors for them. - # TODO: extension.should_install(config) - for extension in all_extensions: - if config[extension.ext_name]['enabled']: + for extension in installed_extensions: + if ext.validate_extension(extension, config): enabled_extensions.append(extension) elif extension.ext_name in config_errors: del config_errors[extension.ext_name] - log_extension_info(all_extensions, enabled_extensions) + log_extension_info(installed_extensions, enabled_extensions) check_config_errors(config_errors) log.setup_log_levels(config) create_file_structures() check_old_locations() - ext.register_gstreamer_elements(enabled_extensions) # Anything that wants to exit after this point must use @@ -99,6 +96,7 @@ def main(): process.stop_remaining_actors() +# TODO: move to config def get_config(files, extensions, overrides): # Helper to get configs, as our config system should not need to know about # extensions. diff --git a/mopidy/ext.py b/mopidy/ext.py index 8a8a1beb..4b3252da 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -112,45 +112,39 @@ def load_extensions(): return installed_extensions -def validate_extensions(installed_extensions): +def validate_extension(extension, config): """Verify extension's dependencies and environment. - :param installed_extensions: list of installed extensions - :returns: list of valid extensions + :param extensions: and extension to check + :param config: config to check enabled status in + :returns: if extensions should be run """ - valid_extensions = [] + logger.debug('Validating extension: %s', extension.ext_name) - for extension in installed_extensions: - logger.debug('Validating extension: %s', extension.ext_name) + if extension.ext_name != extension.entry_point.name: + logger.warning( + 'Disabled extension %(ep)s: entry point name (%(ep)s) ' + 'does not match extension name (%(ext)s)', + {'ep': extension.entry_point.name, 'ext': extension.ext_name}) + return False - if extension.ext_name != extension.entry_point.name: - logger.warning( - 'Disabled extension %(ep)s: entry point name (%(ep)s) ' - 'does not match extension name (%(ext)s)', - {'ep': extension.entry_point.name, 'ext': extension.ext_name}) - continue + try: + extension.entry_point.require() + except pkg_resources.DistributionNotFound as ex: + logger.info( + 'Disabled extension %s: Dependency %s not found', + extension.ext_name, ex) + return False - try: - extension.entry_point.require() - except pkg_resources.DistributionNotFound as ex: - logger.info( - 'Disabled extension %s: Dependency %s not found', - extension.ext_name, ex) - continue + try: + extension.validate_environment() + except exceptions.ExtensionError as ex: + logger.info( + 'Disabled extension %s: %s', extension.ext_name, ex.message) + return False - try: - extension.validate_environment() - except exceptions.ExtensionError as ex: - logger.info( - 'Disabled extension %s: %s', extension.ext_name, ex.message) - continue - - valid_extensions.append(extension) - - names = (e.ext_name for e in valid_extensions) - logger.debug('Valid extensions: %s', ', '.join(names)) - return valid_extensions + return config[extension.ext_name]['enabled'] def register_gstreamer_elements(enabled_extensions): From 78d3888dd1489d58ade0d8c256bcd35f0a0bec19 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 15 Apr 2013 23:09:19 +0200 Subject: [PATCH 08/17] config: Remove ConfigErrors expception --- mopidy/config/__init__.py | 2 -- mopidy/config/schemas.py | 2 -- mopidy/exceptions.py | 18 ------------------ mopidy/utils/process.py | 4 ---- tests/config/config_test.py | 2 +- tests/config/schemas_test.py | 1 - tests/exceptions_test.py | 11 ----------- 7 files changed, 1 insertion(+), 39 deletions(-) diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 3a96b556..2e135ceb 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -94,8 +94,6 @@ def validate(raw_config, schemas, extensions=None): config, errors = _validate(raw_config, schemas + extension_schemas) if errors: - # TODO: raise error instead. - #raise exceptions.ConfigError(errors) for section in errors: for key, error in errors[section].items(): logger.error('Config value %s/%s %s', section, key, error) diff --git a/mopidy/config/schemas.py b/mopidy/config/schemas.py index 90d966d3..1a0c083d 100644 --- a/mopidy/config/schemas.py +++ b/mopidy/config/schemas.py @@ -1,6 +1,5 @@ from __future__ import unicode_literals -from mopidy import exceptions from mopidy.config import types @@ -101,7 +100,6 @@ class ConfigSchema(object): return '\n'.join(lines) - class ExtensionConfigSchema(ConfigSchema): """Sub-classed :class:`ConfigSchema` for use in extensions. diff --git a/mopidy/exceptions.py b/mopidy/exceptions.py index 702eab9f..2c53e3e4 100644 --- a/mopidy/exceptions.py +++ b/mopidy/exceptions.py @@ -16,23 +16,5 @@ class MopidyException(Exception): self._message = message -class ConfigError(MopidyException): - def __init__(self, errors): - self._errors = errors - - def __getitem__(self, key): - return self._errors[key] - - def __iter__(self): - return self._errors.iterkeys() - - @property - def message(self): - lines = [] - for key, msg in self._errors.items(): - lines.append('%s: %s' % (key, msg)) - return '\n'.join(lines) - - class ExtensionError(MopidyException): pass diff --git a/mopidy/utils/process.py b/mopidy/utils/process.py index 24a2e773..c8e3e558 100644 --- a/mopidy/utils/process.py +++ b/mopidy/utils/process.py @@ -8,12 +8,8 @@ import threading from pykka import ActorDeadError from pykka.registry import ActorRegistry -from mopidy import exceptions - - logger = logging.getLogger('mopidy.utils.process') - SIGNALS = dict( (k, v) for v, k in signal.__dict__.iteritems() if v.startswith('SIG') and not v.startswith('SIG_')) diff --git a/tests/config/config_test.py b/tests/config/config_test.py index b112de1f..05b573ec 100644 --- a/tests/config/config_test.py +++ b/tests/config/config_test.py @@ -2,7 +2,7 @@ from __future__ import unicode_literals import mock -from mopidy import config, exceptions +from mopidy import config from tests import unittest, path_to_data_dir diff --git a/tests/config/schemas_test.py b/tests/config/schemas_test.py index f8d908f7..7f4dae4b 100644 --- a/tests/config/schemas_test.py +++ b/tests/config/schemas_test.py @@ -3,7 +3,6 @@ from __future__ import unicode_literals import logging import mock -from mopidy import exceptions from mopidy.config import schemas, types from tests import unittest diff --git a/tests/exceptions_test.py b/tests/exceptions_test.py index daf51a0d..5c244872 100644 --- a/tests/exceptions_test.py +++ b/tests/exceptions_test.py @@ -15,14 +15,3 @@ class ExceptionsTest(unittest.TestCase): def test_extension_error_is_a_mopidy_exception(self): self.assert_(issubclass( exceptions.ExtensionError, exceptions.MopidyException)) - - def test_config_error_is_a_mopidy_exception(self): - self.assert_(issubclass( - exceptions.ConfigError, exceptions.MopidyException)) - - def test_config_error_provides_getitem(self): - exception = exceptions.ConfigError( - {'field1': 'msg1', 'field2': 'msg2'}) - self.assertEqual('msg1', exception['field1']) - self.assertEqual('msg2', exception['field2']) - self.assertItemsEqual(['field1', 'field2'], exception) From 08db8829af57a304c63c29e97602e8dc789606fd Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 15 Apr 2013 23:37:40 +0200 Subject: [PATCH 09/17] config: Move all code to new load function --- mopidy/__main__.py | 34 +++++++++++++--------------------- mopidy/config/__init__.py | 37 ++++++++++--------------------------- mopidy/scanner.py | 10 +++++----- 3 files changed, 28 insertions(+), 53 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index ca99238a..2534657f 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -49,7 +49,7 @@ def main(): try: # Initial config without extensions to bootstrap logging. - logging_config, _ = get_config(config_files, [], config_overrides) + logging_config, _ = config_lib.load(config_files, [], config_overrides) # TODO: setup_logging needs defaults in-case config values are None log.setup_logging( @@ -58,7 +58,7 @@ def main(): installed_extensions = ext.load_extensions() # TODO: wrap config in RO proxy. - config, config_errors = get_config( + config, config_errors = config_lib.load( config_files, installed_extensions, config_overrides) # Filter out disabled extensions and remove any config errors for them. @@ -96,19 +96,6 @@ def main(): process.stop_remaining_actors() -# TODO: move to config -def get_config(files, extensions, overrides): - # Helper to get configs, as our config system should not need to know about - # extensions. - defaults = [config_lib.core_defaults] - defaults.extend(e.get_default_config() for e in extensions) - raw_config = config_lib._load(files, defaults, overrides) - - schemas = config_lib.core_schemas[:] - schemas.extend(e.get_config_schema() for e in extensions) - return config_lib._validate(raw_config, schemas) - - def log_extension_info(all_extensions, enabled_extensions): # TODO: distinguish disabled vs blocked by env? enabled_names = set(e.ext_name for e in enabled_extensions) @@ -185,14 +172,19 @@ def show_config_callback(option, opt, value, parser): overrides = getattr(parser.values, 'overrides', []) extensions = ext.load_extensions() - raw_config = config_lib.load(files, overrides, extensions) - enabled_extensions = ext.filter_enabled_extensions(raw_config, extensions) - config = config_lib.validate( - raw_config, config_lib.core_schemas, enabled_extensions) + enabled_extensions = [] + + config, errors = config_lib.load(files, extensions, overrides) + + for extension in extensions: + if ext.validate_extension(extension, config): + enabled_extensions.append(extension) + elif extension.ext_name in errors: + del errors[extension.ext_name] # TODO: create mopidy.config.format? output = [] - for schema in config_lib.core_schemas: + for schema in config_lib._schemas: options = config.get(schema.name, {}) if not options: continue @@ -205,7 +197,7 @@ def show_config_callback(option, opt, value, parser): options = config.get(schema.name, {}) output.append(schema.format(options)) else: - lines = ['[%s]' % schema.name, 'enabled = false', + lines = ['[%s]' % schema.name, '# Config hidden as extension is disabled'] output.append('\n'.join(lines)) diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 2e135ceb..823323f3 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -12,8 +12,6 @@ from mopidy.utils import path logger = logging.getLogger('mopidy.config') -_config_dir = os.path.dirname(__file__) - _logging_schema = ConfigSchema('logging') _logging_schema['console_format'] = String() _logging_schema['debug_format'] = String() @@ -34,11 +32,7 @@ _proxy_schema['password'] = Secret(optional=True) # NOTE: if multiple outputs ever comes something like LogLevelConfigSchema #_outputs_schema = config.AudioOutputConfigSchema() -#: Config schemas used by mopidy itself. -core_schemas = [_logging_schema, _loglevels_schema, _audio_schema, _proxy_schema] - -#: Config default used by mopidy itself. -core_defaults = io.open(os.path.join(_config_dir, 'default.conf'), 'rb').read() +_schemas = [_logging_schema, _loglevels_schema, _audio_schema, _proxy_schema] def read(config_file): @@ -47,15 +41,19 @@ def read(config_file): return filehandle.read() -def load(files, overrides, extensions=None): +def load(files, extensions, overrides): + # Helper to get configs, as the rest of our config system should not need + # to know about extensions. config_dir = os.path.dirname(__file__) defaults = [read(os.path.join(config_dir, 'default.conf'))] - if extensions: - defaults.extend(e.get_default_config() for e in extensions) - return _load(files, defaults, overrides) + defaults.extend(e.get_default_config() for e in extensions) + raw_config = _load(files, defaults, overrides) + + schemas = _schemas[:] + schemas.extend(e.get_config_schema() for e in extensions) + return _validate(raw_config, schemas) -# TODO: replace load() with this version of API. def _load(files, defaults, overrides): parser = configparser.RawConfigParser() @@ -88,21 +86,6 @@ def _load(files, defaults, overrides): return raw_config -def validate(raw_config, schemas, extensions=None): - # Collect config schemas to validate against - extension_schemas = [e.get_config_schema() for e in extensions or []] - config, errors = _validate(raw_config, schemas + extension_schemas) - - if errors: - for section in errors: - for key, error in errors[section].items(): - logger.error('Config value %s/%s %s', section, key, error) - sys.exit(1) - - return config - - -# TODO: replace validate() with this version of API. def _validate(raw_config, schemas): # Get validated config config = {} diff --git a/mopidy/scanner.py b/mopidy/scanner.py index 226fd410..b30c8ac5 100644 --- a/mopidy/scanner.py +++ b/mopidy/scanner.py @@ -48,17 +48,17 @@ def main(): config_overrides = [] # TODO: decide if we want to avoid this boilerplate some how. - logging_config = config_lib.load(config_files, config_overrides) + # Initial config without extensions to bootstrap logging. + logging_config, _ = config_lib.load(config_files, [], config_overrides) log.setup_root_logger() log.setup_console_logging(logging_config, options.verbosity_level) extensions = ext.load_extensions() - raw_config = config_lib.load(config_files, config_overrides, extensions) - extensions = ext.filter_enabled_extensions(raw_config, extensions) - config = config_lib.validate( - raw_config, config_lib.core_schemas, extensions) + config, errors = config_lib.load(config_files, extensions, config_overrides) log.setup_log_levels(config) + # TODO: missing error checking and other default setup code. + tracks = [] def store(data): From 5a495c590c42519919b861f91ec0a5144805d8eb Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 16 Apr 2013 00:02:31 +0200 Subject: [PATCH 10/17] ext: Do not compound config enabled and validate_extension --- mopidy/__main__.py | 6 ++++-- mopidy/ext.py | 5 ++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 2534657f..71e7fb8a 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -63,7 +63,8 @@ def main(): # Filter out disabled extensions and remove any config errors for them. for extension in installed_extensions: - if ext.validate_extension(extension, config): + enabled = config[extension.ext_name]['enabled'] + if ext.validate_extension(extension) and enabled: enabled_extensions.append(extension) elif extension.ext_name in config_errors: del config_errors[extension.ext_name] @@ -177,7 +178,8 @@ def show_config_callback(option, opt, value, parser): config, errors = config_lib.load(files, extensions, overrides) for extension in extensions: - if ext.validate_extension(extension, config): + enabled = config[extension.ext_name]['enabled'] + if ext.validate_extension(extension) and enabled: enabled_extensions.append(extension) elif extension.ext_name in errors: del errors[extension.ext_name] diff --git a/mopidy/ext.py b/mopidy/ext.py index 4b3252da..325a7800 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -112,11 +112,10 @@ def load_extensions(): return installed_extensions -def validate_extension(extension, config): +def validate_extension(extension): """Verify extension's dependencies and environment. :param extensions: and extension to check - :param config: config to check enabled status in :returns: if extensions should be run """ @@ -144,7 +143,7 @@ def validate_extension(extension, config): 'Disabled extension %s: %s', extension.ext_name, ex.message) return False - return config[extension.ext_name]['enabled'] + return True def register_gstreamer_elements(enabled_extensions): From 211379a01c3054e16463ab90094bef397044442e Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 16 Apr 2013 00:13:57 +0200 Subject: [PATCH 11/17] config: Unknown loglevels should serialize to blank string --- mopidy/config/types.py | 17 ++++++++++------- tests/config/types_test.py | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/mopidy/config/types.py b/mopidy/config/types.py index c43da740..92ccb89a 100644 --- a/mopidy/config/types.py +++ b/mopidy/config/types.py @@ -184,19 +184,22 @@ class LogLevel(ConfigValue): with any casing. """ levels = { - 'critical': logging.CRITICAL, - 'error': logging.ERROR, - 'warning': logging.WARNING, - 'info': logging.INFO, - 'debug': logging.DEBUG, + b'critical': logging.CRITICAL, + b'error': logging.ERROR, + b'warning': logging.WARNING, + b'info': logging.INFO, + b'debug': logging.DEBUG, } def deserialize(self, value): validators.validate_choice(value.lower(), self.levels.keys()) return self.levels.get(value.lower()) - def serialize(self, value): - return dict((v, k) for k, v in self.levels.items()).get(value) + def serialize(self, value, display=False): + lookup = dict((v, k) for k, v in self.levels.items()) + if value in lookup: + return lookup[value] + return b'' class Hostname(ConfigValue): diff --git a/tests/config/types_test.py b/tests/config/types_test.py index a5f2c9dc..e8b026d3 100644 --- a/tests/config/types_test.py +++ b/tests/config/types_test.py @@ -261,7 +261,7 @@ class LogLevelTest(unittest.TestCase): value = types.LogLevel() for name, level in self.levels.items(): self.assertEqual(name, value.serialize(level)) - self.assertIsNone(value.serialize(1337)) + self.assertEqual(b'', value.serialize(1337)) class HostnameTest(unittest.TestCase): From ee40f0385a969fffbe8ec8fc378ce79a0db11d8d Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 16 Apr 2013 00:14:58 +0200 Subject: [PATCH 12/17] config: Remove format from types API --- mopidy/config/schemas.py | 4 ++-- mopidy/config/types.py | 25 +++++++++---------------- tests/config/schemas_test.py | 12 ++++++------ tests/config/types_test.py | 13 ++++++------- 4 files changed, 23 insertions(+), 31 deletions(-) diff --git a/mopidy/config/schemas.py b/mopidy/config/schemas.py index 1a0c083d..ba1d5dad 100644 --- a/mopidy/config/schemas.py +++ b/mopidy/config/schemas.py @@ -96,7 +96,7 @@ class ConfigSchema(object): value = values.get(key) if value is not None: lines.append('%s = %s' % ( - key, self._schema[key].format(value))) + key, self._schema[key].serialize(value, display=True))) return '\n'.join(lines) @@ -142,5 +142,5 @@ class LogLevelConfigSchema(object): for key, value in sorted(values.items()): if value is not None: lines.append('%s = %s' % ( - key, self._config_value.format(value))) + key, self._config_value.serialize(value, display=True))) return '\n'.join(lines) diff --git a/mopidy/config/types.py b/mopidy/config/types.py index 92ccb89a..7e943d27 100644 --- a/mopidy/config/types.py +++ b/mopidy/config/types.py @@ -53,14 +53,10 @@ class ConfigValue(object): """Cast raw string to appropriate type.""" return value - def serialize(self, value): + def serialize(self, value, display=False): """Convert value back to string for saving.""" return bytes(value) - def format(self, value): - """Format value for display.""" - return self.serialize(value) - class String(ConfigValue): """String value. @@ -79,7 +75,7 @@ class String(ConfigValue): return None return value - def serialize(self, value): + def serialize(self, value, display=False): if value is None: return b'' return encode(value) @@ -101,16 +97,13 @@ class Secret(ConfigValue): return None return value - def serialize(self, value): + def serialize(self, value, display=False): if value is None: return b'' + elif display: + return b'********' return value - def format(self, value): - if value is None: - return b'' - return b'********' - class Integer(ConfigValue): """Integer value.""" @@ -147,7 +140,7 @@ class Boolean(ConfigValue): return False raise ValueError('invalid value for boolean: %r' % value) - def serialize(self, value): + def serialize(self, value, display=False): if value: return 'true' else: @@ -173,7 +166,7 @@ class List(ConfigValue): validators.validate_required(values, self._required) return tuple(values) - def serialize(self, value): + def serialize(self, value, display=False): return b'\n ' + b'\n '.join(encode(v) for v in value if v) @@ -208,7 +201,7 @@ class Hostname(ConfigValue): def __init__(self, optional=False): self._required = not optional - def deserialize(self, value): + def deserialize(self, value, display=False): validators.validate_required(value, self._required) if not value.strip(): return None @@ -259,7 +252,7 @@ class Path(ConfigValue): return None return ExpandedPath(value) - def serialize(self, value): + def serialize(self, value, display=False): if isinstance(value, ExpandedPath): return value.original return value diff --git a/tests/config/schemas_test.py b/tests/config/schemas_test.py index 7f4dae4b..8ee9ac50 100644 --- a/tests/config/schemas_test.py +++ b/tests/config/schemas_test.py @@ -17,18 +17,18 @@ class ConfigSchemaTest(unittest.TestCase): self.values = {'bar': '123', 'foo': '456', 'baz': '678'} def test_format(self): - self.schema['foo'].format.return_value = 'qwe' - self.schema['bar'].format.return_value = 'asd' - self.schema['baz'].format.return_value = 'zxc' + self.schema['foo'].serialize.return_value = 'qwe' + self.schema['bar'].serialize.return_value = 'asd' + self.schema['baz'].serialize.return_value = 'zxc' expected = ['[test]', 'foo = qwe', 'bar = asd', 'baz = zxc'] result = self.schema.format(self.values) self.assertEqual('\n'.join(expected), result) def test_format_unkwown_value(self): - self.schema['foo'].format.return_value = 'qwe' - self.schema['bar'].format.return_value = 'asd' - self.schema['baz'].format.return_value = 'zxc' + self.schema['foo'].serialize.return_value = 'qwe' + self.schema['bar'].serialize.return_value = 'asd' + self.schema['baz'].serialize.return_value = 'zxc' self.values['unknown'] = 'rty' result = self.schema.format(self.values) diff --git a/tests/config/types_test.py b/tests/config/types_test.py index e8b026d3..2f3191bf 100644 --- a/tests/config/types_test.py +++ b/tests/config/types_test.py @@ -23,10 +23,9 @@ class ConfigValueTest(unittest.TestCase): value = types.ConfigValue() self.assertIsInstance(value.serialize(object()), bytes) - def test_format_uses_serialize(self): + def test_serialize_supports_display(self): value = types.ConfigValue() - obj = object() - self.assertEqual(value.serialize(obj), value.format(obj)) + self.assertIsInstance(value.serialize(object(), display=True), bytes) class StringTest(unittest.TestCase): @@ -117,15 +116,15 @@ class SecretTest(unittest.TestCase): self.assertIsInstance(result, bytes) self.assertEqual(b'', result) - def test_format_masks_value(self): + def test_serialize_for_display_masks_value(self): value = types.Secret() - result = value.format('s3cret') + result = value.serialize('s3cret', display=True) self.assertIsInstance(result, bytes) self.assertEqual(b'********', result) - def test_format_none(self): + def test_serialize_none_for_display(self): value = types.Secret() - result = value.format(None) + result = value.serialize(None, display=True) self.assertIsInstance(result, bytes) self.assertEqual(b'', result) From f0131fdc9312a07e8b0e9c0f9d35f9817f938ab2 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 16 Apr 2013 21:21:09 +0200 Subject: [PATCH 13/17] config: Fix serialization of none --- mopidy/config/types.py | 6 ++++-- tests/config/types_test.py | 21 ++++++++++++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/mopidy/config/types.py b/mopidy/config/types.py index 7e943d27..f8614efd 100644 --- a/mopidy/config/types.py +++ b/mopidy/config/types.py @@ -55,6 +55,8 @@ class ConfigValue(object): def serialize(self, value, display=False): """Convert value back to string for saving.""" + if value is None: + return b'' return bytes(value) @@ -142,9 +144,9 @@ class Boolean(ConfigValue): def serialize(self, value, display=False): if value: - return 'true' + return b'true' else: - return 'false' + return b'false' class List(ConfigValue): diff --git a/tests/config/types_test.py b/tests/config/types_test.py index 2f3191bf..de800e98 100644 --- a/tests/config/types_test.py +++ b/tests/config/types_test.py @@ -23,6 +23,12 @@ class ConfigValueTest(unittest.TestCase): value = types.ConfigValue() self.assertIsInstance(value.serialize(object()), bytes) + def test_serialize_none(self): + value = types.ConfigValue() + result = value.serialize(None) + self.assertIsInstance(result, bytes) + self.assertEqual(b'', result) + def test_serialize_supports_display(self): value = types.ConfigValue() self.assertIsInstance(value.serialize(object(), display=True), bytes) @@ -177,10 +183,19 @@ class BooleanTest(unittest.TestCase): self.assertRaises(ValueError, value.deserialize, 'sure') self.assertRaises(ValueError, value.deserialize, '') - def test_serialize(self): + def test_serialize_true(self): value = types.Boolean() - self.assertEqual('true', value.serialize(True)) - self.assertEqual('false', value.serialize(False)) + result = value.serialize(True) + self.assertEqual(b'true', result) + self.assertIsInstance(result, bytes) + + def test_serialize_false(self): + value = types.Boolean() + result = value.serialize(False) + self.assertEqual(b'false', result) + self.assertIsInstance(result, bytes) + + # TODO: test None or other invalid values into serialize? class ListTest(unittest.TestCase): From d8f68863113af555314005a01a9e1b8073437be9 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 16 Apr 2013 22:19:34 +0200 Subject: [PATCH 14/17] config: Set missing/invalid keys to none --- mopidy/config/schemas.py | 3 +++ tests/config/config_test.py | 4 ++-- tests/config/schemas_test.py | 38 +++++++++++++++++++++++++----------- 3 files changed, 32 insertions(+), 13 deletions(-) diff --git a/mopidy/config/schemas.py b/mopidy/config/schemas.py index ba1d5dad..66da481b 100644 --- a/mopidy/config/schemas.py +++ b/mopidy/config/schemas.py @@ -75,10 +75,12 @@ class ConfigSchema(object): if suggestion: errors[key] += ' Did you mean %s?' % suggestion except ValueError as e: # deserialization failed + result[key] = None errors[key] = str(e) for key in self._schema: if key not in result and key not in errors: + result[key] = None errors[key] = 'config key not found.' return result, errors @@ -131,6 +133,7 @@ class LogLevelConfigSchema(object): try: result[key] = self._config_value.deserialize(value) except ValueError as e: # deserialization failed + result[key] = None errors[key] = str(e) return result, errors diff --git a/tests/config/config_test.py b/tests/config/config_test.py index 05b573ec..cd708d85 100644 --- a/tests/config/config_test.py +++ b/tests/config/config_test.py @@ -69,7 +69,7 @@ class ValidateTest(unittest.TestCase): def test_empty_config_single_schema(self): conf, errors = config._validate({}, [self.schema]) - self.assertEqual({}, conf) + self.assertEqual({'foo': {'bar': None}}, conf) self.assertEqual({'foo': {'bar': 'config key not found.'}}, errors) def test_config_single_schema(self): @@ -83,7 +83,7 @@ class ValidateTest(unittest.TestCase): self.schema['bar'] = mock.Mock() self.schema['bar'].deserialize.side_effect = ValueError('bad') conf, errors = config._validate(raw_config, [self.schema]) - self.assertEqual({}, conf) + self.assertEqual({'foo': {'bar': None}}, conf) self.assertEqual({'foo': {'bar': 'bad'}}, errors) # TODO: add more tests diff --git a/tests/config/schemas_test.py b/tests/config/schemas_test.py index 8ee9ac50..c3ce2f4d 100644 --- a/tests/config/schemas_test.py +++ b/tests/config/schemas_test.py @@ -5,7 +5,7 @@ import mock from mopidy.config import schemas, types -from tests import unittest +from tests import unittest, any_unicode class ConfigSchemaTest(unittest.TestCase): @@ -41,30 +41,42 @@ class ConfigSchemaTest(unittest.TestCase): del self.values['foo'] result, errors = self.schema.deserialize(self.values) - self.assertIn('not found', errors['foo']) - self.assertItemsEqual(['bar', 'baz'], result.keys()) + self.assertEqual({'foo': any_unicode}, errors) + self.assertIsNone(result.pop('foo')) + self.assertIsNotNone(result.pop('bar')) + self.assertIsNotNone(result.pop('baz')) + self.assertEqual({}, result) def test_deserialize_with_extra_value(self): self.values['extra'] = '123' result, errors = self.schema.deserialize(self.values) - self.assertIn('unknown', errors['extra']) - self.assertItemsEqual(['foo', 'bar', 'baz'], result.keys()) + self.assertEqual({'extra': any_unicode}, errors) + self.assertIsNotNone(result.pop('foo')) + self.assertIsNotNone(result.pop('bar')) + self.assertIsNotNone(result.pop('baz')) + self.assertEqual({}, result) def test_deserialize_with_deserialization_error(self): self.schema['foo'].deserialize.side_effect = ValueError('failure') + result, errors = self.schema.deserialize(self.values) - self.assertIn('failure', errors['foo']) - self.assertItemsEqual(['bar', 'baz'], result.keys()) + self.assertEqual({'foo': 'failure'}, errors) + self.assertIsNone(result.pop('foo')) + self.assertIsNotNone(result.pop('bar')) + self.assertIsNotNone(result.pop('baz')) + self.assertEqual({}, result) def test_deserialize_with_multiple_deserialization_errors(self): self.schema['foo'].deserialize.side_effect = ValueError('failure') self.schema['bar'].deserialize.side_effect = ValueError('other') result, errors = self.schema.deserialize(self.values) - self.assertIn('failure', errors['foo']) - self.assertIn('other', errors['bar']) - self.assertItemsEqual(['baz'], result.keys()) + self.assertEqual({'foo': 'failure', 'bar': 'other'}, errors) + self.assertIsNone(result.pop('foo')) + self.assertIsNone(result.pop('bar')) + self.assertIsNotNone(result.pop('baz')) + self.assertEqual({}, result) def test_deserialize_deserialization_unknown_and_missing_errors(self): self.values['extra'] = '123' @@ -76,7 +88,11 @@ class ConfigSchemaTest(unittest.TestCase): self.assertNotIn('foo', errors) self.assertIn('failure', errors['bar']) self.assertIn('not found', errors['baz']) - self.assertItemsEqual(['foo'], result.keys()) + + self.assertNotIn('unknown', result) + self.assertIn('foo', result) + self.assertIsNone(result['bar']) + self.assertIsNone(result['baz']) class ExtensionConfigSchemaTest(unittest.TestCase): From cde84748f70c5d581a2b2ac3de80197f7bc50871 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 16 Apr 2013 22:36:58 +0200 Subject: [PATCH 15/17] config: Create config.format and switch to just serialize on schemas --- mopidy/__main__.py | 35 ++++++++++------------------------- mopidy/config/__init__.py | 24 ++++++++++++++++++++++++ mopidy/config/schemas.py | 36 ++++++++++++------------------------ tests/config/schemas_test.py | 25 ------------------------- 4 files changed, 46 insertions(+), 74 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 71e7fb8a..15596c3e 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -177,33 +177,18 @@ def show_config_callback(option, opt, value, parser): config, errors = config_lib.load(files, extensions, overrides) + # Clear out any config for disabled extensions. for extension in extensions: - enabled = config[extension.ext_name]['enabled'] - if ext.validate_extension(extension) and enabled: - enabled_extensions.append(extension) - elif extension.ext_name in errors: - del errors[extension.ext_name] + if not ext.validate_extension(extension): + config[extension.ext_name] = {b'enabled': False} + errors[extension.ext_name] = { + b'enabled': b'extension disabled its self.'} + elif not config[extension.ext_name]['enabled']: + config[extension.ext_name] = {b'enabled': False} + errors[extension.ext_name] = { + b'enabled': b'extension disabled by config.'} - # TODO: create mopidy.config.format? - output = [] - for schema in config_lib._schemas: - options = config.get(schema.name, {}) - if not options: - continue - output.append(schema.format(options)) - - for extension in extensions: - schema = extension.get_config_schema() - - if extension in enabled_extensions: - options = config.get(schema.name, {}) - output.append(schema.format(options)) - else: - lines = ['[%s]' % schema.name, - '# Config hidden as extension is disabled'] - output.append('\n'.join(lines)) - - print '\n\n'.join(output) + print config_lib.format(config, extensions, errors) sys.exit(0) diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 823323f3..1cdcfa9b 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -54,6 +54,14 @@ def load(files, extensions, overrides): return _validate(raw_config, schemas) +def format(config, extensions, comments=None, display=True): + # Helper to format configs, as the rest of our config system should not + # need to know about extensions. + schemas = _schemas[:] + schemas.extend(e.get_config_schema() for e in extensions) + return _format(config, comments or {}, schemas, display) + + def _load(files, defaults, overrides): parser = configparser.RawConfigParser() @@ -100,6 +108,22 @@ def _validate(raw_config, schemas): return config, errors +def _format(config, comments, schemas, display): + output = [] + for schema in schemas: + serialized = schema.serialize(config.get(schema.name, {}), display=display) + output.append(b'[%s]' % schema.name) + for key, value in serialized.items(): + comment = comments.get(schema.name, {}).get(key, b'') + output.append(b'%s =' % key) + if value is not None: + output[-1] += b' ' + value + if comment: + output[-1] += b' # ' + comment.capitalize() + output.append(b'') + return b'\n'.join(output) + + def parse_override(override): """Parse ``section/key=value`` command line overrides""" section, remainder = override.split('/', 1) diff --git a/mopidy/config/schemas.py b/mopidy/config/schemas.py index 66da481b..9f10c41f 100644 --- a/mopidy/config/schemas.py +++ b/mopidy/config/schemas.py @@ -1,5 +1,7 @@ from __future__ import unicode_literals +import collections + from mopidy.config import types @@ -85,21 +87,12 @@ class ConfigSchema(object): return result, errors - def serialize(self, values): - pass - - def format(self, values): - """Returns the schema as a config section with the given ``values`` - filled in""" - # TODO: should the output be encoded utf-8 since we use that in - # serialize for strings? - lines = ['[%s]' % self.name] + def serialize(self, values, display=False): + result = getattr(collections, 'OrderedDict', dict)() # TODO: 2.6 cleanup for key in self._order: - value = values.get(key) - if value is not None: - lines.append('%s = %s' % ( - key, self._schema[key].serialize(value, display=True))) - return '\n'.join(lines) + if key in values: + result[key] = self._schema[key].serialize(values[key], display) + return result class ExtensionConfigSchema(ConfigSchema): @@ -137,13 +130,8 @@ class LogLevelConfigSchema(object): errors[key] = str(e) return result, errors - def serialize(self, values): - pass - - def format(self, values): - lines = ['[%s]' % self.name] - for key, value in sorted(values.items()): - if value is not None: - lines.append('%s = %s' % ( - key, self._config_value.serialize(value, display=True))) - return '\n'.join(lines) + def serialize(self, values, display=False): + result = getattr(collections, 'OrderedDict', dict)() # TODO: 2.6 cleanup + for key in sorted(values.keys()): + result[key] = self._config_value.serialize(values[key], display) + return result diff --git a/tests/config/schemas_test.py b/tests/config/schemas_test.py index c3ce2f4d..a0c519f4 100644 --- a/tests/config/schemas_test.py +++ b/tests/config/schemas_test.py @@ -16,24 +16,6 @@ class ConfigSchemaTest(unittest.TestCase): self.schema['baz'] = mock.Mock() self.values = {'bar': '123', 'foo': '456', 'baz': '678'} - def test_format(self): - self.schema['foo'].serialize.return_value = 'qwe' - self.schema['bar'].serialize.return_value = 'asd' - self.schema['baz'].serialize.return_value = 'zxc' - - expected = ['[test]', 'foo = qwe', 'bar = asd', 'baz = zxc'] - result = self.schema.format(self.values) - self.assertEqual('\n'.join(expected), result) - - def test_format_unkwown_value(self): - self.schema['foo'].serialize.return_value = 'qwe' - self.schema['bar'].serialize.return_value = 'asd' - self.schema['baz'].serialize.return_value = 'zxc' - self.values['unknown'] = 'rty' - - result = self.schema.format(self.values) - self.assertNotIn('unknown = rty', result) - def test_deserialize(self): self.schema.deserialize(self.values) @@ -109,13 +91,6 @@ class LogLevelConfigSchemaTest(unittest.TestCase): self.assertEqual(logging.DEBUG, result['foo.bar']) self.assertEqual(logging.INFO, result['baz']) - def test_format(self): - schema = schemas.LogLevelConfigSchema('test') - values = {'foo.bar': logging.DEBUG, 'baz': logging.INFO} - expected = ['[test]', 'baz = info', 'foo.bar = debug'] - result = schema.format(values) - self.assertEqual('\n'.join(expected), result) - class DidYouMeanTest(unittest.TestCase): def testSuggestoins(self): From 134faddb9ed0e729c7ea74987e622e4e8e580adb Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 16 Apr 2013 22:48:40 +0200 Subject: [PATCH 16/17] ext: Typo in docstring --- mopidy/ext.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/ext.py b/mopidy/ext.py index 325a7800..85dceee4 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -115,8 +115,8 @@ def load_extensions(): def validate_extension(extension): """Verify extension's dependencies and environment. - :param extensions: and extension to check - :returns: if extensions should be run + :param extensions: an extension to check + :returns: if extension should be run """ logger.debug('Validating extension: %s', extension.ext_name) From 765f3c5d3ca1e65c974f1306a51ebb19529ca871 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 16 Apr 2013 23:25:30 +0200 Subject: [PATCH 17/17] config: flake8 --- mopidy/__main__.py | 2 -- mopidy/config/__init__.py | 1 - mopidy/config/schemas.py | 7 +++++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 15596c3e..022a59ba 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -173,8 +173,6 @@ def show_config_callback(option, opt, value, parser): overrides = getattr(parser.values, 'overrides', []) extensions = ext.load_extensions() - enabled_extensions = [] - config, errors = config_lib.load(files, extensions, overrides) # Clear out any config for disabled extensions. diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index 1cdcfa9b..6438f975 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -4,7 +4,6 @@ import ConfigParser as configparser import io import logging import os.path -import sys from mopidy.config.schemas import * from mopidy.config.types import * diff --git a/mopidy/config/schemas.py b/mopidy/config/schemas.py index 9f10c41f..ba91161c 100644 --- a/mopidy/config/schemas.py +++ b/mopidy/config/schemas.py @@ -4,6 +4,9 @@ import collections from mopidy.config import types +# TODO: 2.6 cleanup (#344). +ordered_dict = getattr(collections, 'OrderedDict', dict) + def _did_you_mean(name, choices): """Suggest most likely setting based on levenshtein.""" @@ -88,7 +91,7 @@ class ConfigSchema(object): return result, errors def serialize(self, values, display=False): - result = getattr(collections, 'OrderedDict', dict)() # TODO: 2.6 cleanup + result = ordered_dict() # TODO: 2.6 cleanup (#344). for key in self._order: if key in values: result[key] = self._schema[key].serialize(values[key], display) @@ -131,7 +134,7 @@ class LogLevelConfigSchema(object): return result, errors def serialize(self, values, display=False): - result = getattr(collections, 'OrderedDict', dict)() # TODO: 2.6 cleanup + result = ordered_dict() # TODO: 2.6 cleanup (#344) for key in sorted(values.keys()): result[key] = self._config_value.serialize(values[key], display) return result