checks.py: show warning message for alias option

Show warning message for any alias option find in config instance.
Example warning message:

WARNING: Config option 'product_name' is deprecated and now an alias to
'release_name', please use 'release_name' instead. In:
{'release_name': 'dummy product', 'product_name': 'dummy product'}

Signed-off-by: Qixiang Wan <qwan@redhat.com>
This commit is contained in:
Qixiang Wan 2017-03-23 09:18:15 -06:00
parent 9784961568
commit 2aacefd9cd
3 changed files with 62 additions and 45 deletions

View File

@ -35,6 +35,7 @@ When a new config option is added, the schema must be updated (see the
``CONFIG_DEPS`` mapping. ``CONFIG_DEPS`` mapping.
""" """
import contextlib
import os.path import os.path
import platform import platform
import jsonschema import jsonschema
@ -196,6 +197,10 @@ def validate(config):
for error in validator.iter_errors(config): for error in validator.iter_errors(config):
if isinstance(error, ConfigDeprecation): if isinstance(error, ConfigDeprecation):
warnings.append(REMOVED.format('.'.join(error.path), error.message)) warnings.append(REMOVED.format('.'.join(error.path), error.message))
elif isinstance(error, ConfigOptionWarning):
warnings.append(error.message)
elif isinstance(error, ConfigOptionError):
errors.append(error.message)
elif not error.path and error.validator == 'additionalProperties': elif not error.path and error.validator == 'additionalProperties':
allowed_keys = set(error.schema['properties'].keys()) allowed_keys = set(error.schema['properties'].keys())
used_keys = set(error.instance.keys()) used_keys = set(error.instance.keys())
@ -241,61 +246,60 @@ def _extend_with_default_and_alias(validator_class):
validate_required = validator_class.VALIDATORS['required'] validate_required = validator_class.VALIDATORS['required']
validate_additional_properties = validator_class.VALIDATORS['additionalProperties'] validate_additional_properties = validator_class.VALIDATORS['additionalProperties']
def _replace_alias(properties, instance, schema): @contextlib.contextmanager
def _hook_errors(properties, instance, schema):
""" """
If 'alias' is defined for a property, and the property is not present Hook the instance and yield errors and warnings.
in instance, add the property with value from the alias property to
instance before remove the alias property. If both the propery and its
alias are present, it will yield an error.
""" """
errors = []
for property, subschema in properties.iteritems(): for property, subschema in properties.iteritems():
# update instance for alias option
# If alias option for the property is present and property is not specified,
# update the property in instance with value from alias option.
if "alias" in subschema: if "alias" in subschema:
if property in instance and subschema['alias'] in instance: if subschema['alias'] in instance:
# the order of validators is in random, so we remove the alias msg = "WARNING: Config option '%s' is deprecated and now an alias to '%s', " \
# property at the first time when it's found, then validators "please use '%s' instead. " \
# won't raise the same error later. "In:\n%s" % (subschema['alias'], property, property, instance)
errors.append(ConfigOptionWarning(msg))
if property in instance:
msg = "ERROR: Config option '%s' is an alias of '%s', only one can be used. In:\n%s" \
% (subschema['alias'], property, instance)
errors.append(ConfigOptionError(msg))
instance.pop(subschema['alias']) instance.pop(subschema['alias'])
yield jsonschema.ValidationError( else:
"%s is an alias of %s, only one can be used." % (
subschema['alias'], property)
)
if property not in instance and subschema['alias'] in instance:
instance.setdefault(property, instance.pop(subschema['alias'])) instance.setdefault(property, instance.pop(subschema['alias']))
yield errors
def set_defaults_and_aliases(validator, properties, instance, schema): def _set_defaults(validator, properties, instance, schema):
""" """
Assign default values to options that have them defined and are not Assign default values to options that have them defined and are not
specified. And if a property has 'alias' defined and the property is specified.
not specified, look for the alias property and copy alias property's
value to that property before remove the alias property.
""" """
for property, subschema in properties.iteritems(): for property, subschema in properties.iteritems():
if "default" in subschema and property not in instance: if "default" in subschema and property not in instance:
instance.setdefault(property, subschema["default"]) instance.setdefault(property, subschema["default"])
for error in _replace_alias(properties, instance, schema): with _hook_errors(properties, instance, schema) as errors:
for error in errors:
yield error yield error
for error in validate_properties(validator, properties, instance, schema): for error in validate_properties(validator, properties, instance, schema):
yield error yield error
def ignore_alias_properties(validator, aP, instance, schema): def _validate_additional_properties(validator, aP, instance, schema):
"""
If there is a property has alias defined in schema, and the property is not
present in instance, set the property with the value of alias property,
remove alias property from instance.
"""
properties = schema.get("properties", {}) properties = schema.get("properties", {})
for error in _replace_alias(properties, instance, schema): with _hook_errors(properties, instance, schema) as errors:
for error in errors:
yield error yield error
for error in validate_additional_properties(validator, aP, instance, schema): for error in validate_additional_properties(validator, aP, instance, schema):
yield error yield error
def validate_required_with_alias(validator, required, instance, schema): def _validate_required(validator, required, instance, schema):
properties = schema.get("properties", {}) properties = schema.get("properties", {})
for error in _replace_alias(properties, instance, schema): with _hook_errors(properties, instance, schema) as errors:
for error in errors:
yield error yield error
for error in validate_required(validator, required, instance, schema): for error in validate_required(validator, required, instance, schema):
@ -323,11 +327,11 @@ def _extend_with_default_and_alias(validator_class):
yield error yield error
return jsonschema.validators.extend( return jsonschema.validators.extend(
validator_class, {"properties": set_defaults_and_aliases, validator_class, {"properties": _set_defaults,
"deprecated": error_on_deprecated, "deprecated": error_on_deprecated,
"type": validate_regex_type, "type": validate_regex_type,
"required": validate_required_with_alias, "required": _validate_required,
"additionalProperties": ignore_alias_properties}, "additionalProperties": _validate_additional_properties},
) )
@ -335,6 +339,14 @@ class ConfigDeprecation(jsonschema.exceptions.ValidationError):
pass pass
class ConfigOptionWarning(jsonschema.exceptions.ValidationError):
pass
class ConfigOptionError(jsonschema.exceptions.ValidationError):
pass
def _make_schema(): def _make_schema():
return { return {
"$schema": "http://json-schema.org/draft-04/schema#", "$schema": "http://json-schema.org/draft-04/schema#",

View File

@ -238,7 +238,8 @@ class TestSchemaValidator(unittest.TestCase):
config = self._load_conf_from_string(string) config = self._load_conf_from_string(string)
errors, warnings = checks.validate(config) errors, warnings = checks.validate(config)
self.assertEqual(len(errors), 0) self.assertEqual(len(errors), 0)
self.assertEqual(len(warnings), 0) self.assertEqual(len(warnings), 1)
self.assertRegexpMatches(warnings[0], r"^WARNING: Config option 'product_name' is deprecated and now an alias to 'release_name'.*")
self.assertEqual(config.get("release_name", None), "dummy product") self.assertEqual(config.get("release_name", None), "dummy product")
@mock.patch('pungi.checks._make_schema') @mock.patch('pungi.checks._make_schema')
@ -285,7 +286,8 @@ class TestSchemaValidator(unittest.TestCase):
config = self._load_conf_from_string(string) config = self._load_conf_from_string(string)
errors, warnings = checks.validate(config) errors, warnings = checks.validate(config)
self.assertEqual(len(errors), 0) self.assertEqual(len(errors), 0)
self.assertEqual(len(warnings), 0) self.assertEqual(len(warnings), 1)
self.assertRegexpMatches(warnings[0], r"^WARNING: Config option 'product_name' is deprecated and now an alias to 'release_name'.*")
self.assertEqual(config.get("release_name", None), "dummy product") self.assertEqual(config.get("release_name", None), "dummy product")
@mock.patch('pungi.checks._make_schema') @mock.patch('pungi.checks._make_schema')
@ -309,8 +311,9 @@ class TestSchemaValidator(unittest.TestCase):
config = self._load_conf_from_string(string) config = self._load_conf_from_string(string)
errors, warnings = checks.validate(config) errors, warnings = checks.validate(config)
self.assertEqual(len(errors), 1) self.assertEqual(len(errors), 1)
self.assertIn('Failed validation in : product_name is an alias of release_name, only one can be used.', errors) self.assertRegexpMatches(errors[0], r"^ERROR: Config option 'product_name' is an alias of 'release_name', only one can be used.*")
self.assertEqual(len(warnings), 0) self.assertEqual(len(warnings), 1)
self.assertRegexpMatches(warnings[0], r"^WARNING: Config option 'product_name' is deprecated and now an alias to 'release_name'.*")
self.assertEqual(config.get("release_name", None), "dummy product") self.assertEqual(config.get("release_name", None), "dummy product")
@mock.patch('pungi.checks._make_schema') @mock.patch('pungi.checks._make_schema')
@ -348,7 +351,9 @@ class TestSchemaValidator(unittest.TestCase):
config = self._load_conf_from_string(string) config = self._load_conf_from_string(string)
errors, warnings = checks.validate(config) errors, warnings = checks.validate(config)
self.assertEqual(len(errors), 0) self.assertEqual(len(errors), 0)
self.assertEqual(len(warnings), 0) self.assertEqual(len(warnings), 2)
self.assertRegexpMatches(warnings[0], r"^WARNING: Config option '.+' is deprecated and now an alias to '.+'.*")
self.assertRegexpMatches(warnings[1], r"^WARNING: Config option '.+' is deprecated and now an alias to '.+'.*")
self.assertEqual(config.get("release_name", None), "dummy product") self.assertEqual(config.get("release_name", None), "dummy product")
self.assertEqual(config.get("foophase", {}).get("repo", None), "http://www.exampe.com/os") self.assertEqual(config.get("foophase", {}).get("repo", None), "http://www.exampe.com/os")

View File

@ -273,7 +273,7 @@ class OstreeConfigTestCase(ConfigTestCase):
"x86_64": { "x86_64": {
"treefile": "fedora-atomic-docker-host.json", "treefile": "fedora-atomic-docker-host.json",
"config_url": "https://git.fedorahosted.org/git/fedora-atomic.git", "config_url": "https://git.fedorahosted.org/git/fedora-atomic.git",
"source_repo_from": "Everything", "repo_from": "Everything",
"ostree_repo": "/mnt/koji/compose/atomic/Rawhide/" "ostree_repo": "/mnt/koji/compose/atomic/Rawhide/"
} }
}) })
@ -298,7 +298,7 @@ class OstreeInstallerConfigTestCase(ConfigTestCase):
ostree_installer=[ ostree_installer=[
("^Atomic$", { ("^Atomic$", {
"x86_64": { "x86_64": {
"source_repo_from": "Everything", "repo_from": "Everything",
"release": None, "release": None,
"installpkgs": ["fedora-productimg-atomic"], "installpkgs": ["fedora-productimg-atomic"],
"add_template": ["/spin-kickstarts/atomic-installer/lorax-configure-repo.tmpl"], "add_template": ["/spin-kickstarts/atomic-installer/lorax-configure-repo.tmpl"],
@ -326,7 +326,7 @@ class OstreeInstallerConfigTestCase(ConfigTestCase):
ostree_installer=[ ostree_installer=[
("^Atomic$", { ("^Atomic$", {
"x86_64": { "x86_64": {
"source_repo_from": "Everything", "repo_from": "Everything",
"release": None, "release": None,
"installpkgs": ["fedora-productimg-atomic"], "installpkgs": ["fedora-productimg-atomic"],
"add_template": ["/spin-kickstarts/atomic-installer/lorax-configure-repo.tmpl"], "add_template": ["/spin-kickstarts/atomic-installer/lorax-configure-repo.tmpl"],