config: Return convereted values and errors

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:
+        for section in errors:
-            logger.error(error)
+            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:
+        values = raw_config.get(schema.name, {})
-            items = raw_config[schema.name].items()
+        result, error = schema.convert(values)
-            config[schema.name] = schema.convert(items)
+        if error:
-        except KeyError:
+            errors[schema.name] = error
-            errors.append('%s: section not found.' % schema.name)
+        config[schema.name] = result
-        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
     return config, errors
 
 

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):
+    def convert(self, values):
-        """Validates the given ``items`` using the config schema and returns
+        """Validates the given ``values`` using the config schema.
-        clean values"""
-        errors = {}
-        values = {}
 
-        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:
+        return result, errors
-            raise exceptions.ConfigError(errors)
-        return values
 
 
 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():
+                result[key] = self._config_value.deserialize(value)
-                    values[key] = self._config_value.deserialize(value)
             except ValueError as e:  # deserialization failed
                 errors[key] = str(e)
-
+        return result, errors
-        if errors:
-            raise exceptions.ConfigError(errors)
-        return values

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:
+        result, errors = self.schema.convert(self.values)
-            self.schema.convert(self.values.items())
+        self.assertIn('not found', errors['foo'])
-
+        self.assertItemsEqual(['bar', 'baz'], result.keys())
-        self.assertIn('not found', cm.exception['foo'])
 
     def test_convert_with_extra_value(self):
         self.values['extra'] = '123'
 
-        with self.assertRaises(exceptions.ConfigError) as cm:
+        result, errors = self.schema.convert(self.values)
-            self.schema.convert(self.values.items())
+        self.assertIn('unknown', errors['extra'])
-
+        self.assertItemsEqual(['foo', 'bar', 'baz'], result.keys())
-        self.assertIn('unknown', cm.exception['extra'])
 
     def test_convert_with_deserialization_error(self):
         self.schema['foo'].deserialize.side_effect = ValueError('failure')
 
-        with self.assertRaises(exceptions.ConfigError) as cm:
+        result, errors = self.schema.convert(self.values)
-            self.schema.convert(self.values.items())
+        self.assertIn('failure', errors['foo'])
-
+        self.assertItemsEqual(['bar', 'baz'], result.keys())
-        self.assertIn('failure', cm.exception['foo'])
 
     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:
+        result, errors = self.schema.convert(self.values)
-            self.schema.convert(self.values.items())
+        self.assertIn('failure', errors['foo'])
-
+        self.assertIn('other', errors['bar'])
-        self.assertIn('failure', cm.exception['foo'])
+        self.assertItemsEqual(['baz'], result.keys())
-        self.assertIn('other', cm.exception['bar'])
 
     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:
+        result, errors = self.schema.convert(self.values)
-            self.schema.convert(self.values.items())
+        self.assertIn('unknown', errors['extra'])
-
+        self.assertNotIn('foo', errors)
-        self.assertIn('unknown', cm.exception['extra'])
+        self.assertIn('failure', errors['bar'])
-        self.assertNotIn('foo', cm.exception)
+        self.assertIn('not found', errors['baz'])
-        self.assertIn('failure', cm.exception['bar'])
+        self.assertItemsEqual(['foo'], result.keys())
-        self.assertIn('not found', cm.exception['baz'])
 
 
 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'])