openstack
/
sushy
Code Issues Proposed changes

Relax required Redfish fields handling 

Redfish defines some of the fields in its JSON schemas as
mandatory. However, some implementations ignore this requirement
and occasionally omit some required fields in the Redfish document
tree they produce.

Failing the whole Redfish interaction basing solely on the absence
of a required (bit non-essential) field makes sushy perfect, but not
exactly practical.

This patch changes to semantics of the `default` parameter in
``Field` constructor in a way that it can inhibit otherwise fatal
failure when a select of required attributes are missing.

Along this mis/feature, some non-essential fields in Redfish
message registry have been made required and defaulted effectively
making them non-required.

Change-Id: I637f11ff9ceab398077eae19db83db396356c8dc
Story: 2006641
Task: 38362
(cherry picked from commit 8440eb2826)
This commit is contained in:
Ilya Etingof 2020-01-22 16:44:19 +01:00 committed by Julia Kreger
parent 4e2eb5a405
commit 3f08acc803
3 changed files with 35 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
sushy/resources/base.py
View File

@ -49,10 +49,9 @@ class Field(object):
:param path: JSON field to fetch the value from. Either a string,
or a list of strings in case of a nested field.
:param required: whether this field is required. Missing required
fields result in MissingAttributeError.
:param required: whether this field is required. Missing required,
but not defaulted, fields result in MissingAttributeError.
:param default: the default value to use when the field is missing.
Only has effect when the field is not required.
:param adapter: a function to call to transform and/or validate
the received value. UnicodeError, ValueError or TypeError from
this call are reraised as MalformedAttributeError.
@ -89,7 +88,8 @@ class Field(object):
:param resource: ResourceBase instance for which the field is loaded.
:param nested_in: parent resource path (for error reporting only),
must be a list of strings or None.
:raises: MissingAttributeError if a required field is missing.
:raises: MissingAttributeError if a required field is missing
and not defaulted.
:raises: MalformedAttributeError on invalid field value or type.
:returns: loaded and verified value
"""
@ -103,12 +103,18 @@ class Field(object):
except KeyError:
if self._required:
path = (nested_in or []) + self._path
raise exceptions.MissingAttributeError(
attribute='/'.join(path),
resource=resource.path)
else:
# Do not run the adapter on the default value
return self._default
if self._default is None:
raise exceptions.MissingAttributeError(
attribute='/'.join(path),
resource=resource.path)
logging.warning(
'Applying default "%s" on required, but missing '
'attribute "%s"' % (self._default, path))
# Do not run the adapter on the default value
return self._default
# NOTE(etingof): this is just to account for schema violation
if item is None:
@ -262,11 +268,10 @@ class MappedField(Field):
a string or a list of string. In the latter case, the value will
be fetched from a nested object.
:param mapping: a mapping to take values from.
:param required: whether this field is required. Missing required
fields result in MissingAttributeError.
:param required: whether this field is required. Missing required,
but not defaulted, fields result in MissingAttributeError.
:param default: the default value to use when the field is missing.
Only has effect when the field is not required. This value is not
matched against the mapping.
This value is not matched against the mapping.
"""
if not isinstance(mapping, collectionsAbc.Mapping):
raise TypeError("The mapping argument must be a mapping")
@ -289,10 +294,9 @@ class MappedListField(Field):
:param field: JSON field to fetch the list of values from.
:param mapping: a mapping for the list elements.
:param required: whether this field is required. Missing required
fields result in MissingAttributeError.
:param required: whether this field is required. Missing required,
but not defaulted, fields result in MissingAttributeError.
:param default: the default value to use when the field is missing.
Only has effect when the field is not required.
"""
if not isinstance(mapping, collectionsAbc.Mapping):
raise TypeError("The mapping argument must be a mapping")

5
sushy/resources/registry/message_registry.py
View File

@ -21,7 +21,7 @@ from sushy.resources import mappings as res_maps
class MessageDictionaryField(base.DictionaryField):
description = base.Field('Description', required=False)
description = base.Field('Description', required=True, default='')
"""Indicates how and when the message is returned by the Redfish service"""
message = base.Field('Message', required=True)
@ -47,7 +47,8 @@ class MessageDictionaryField(base.DictionaryField):
severity = base.MappedField('Severity',
res_maps.SEVERITY_VALUE_MAP,
default=res_cons.SEVERITY_CRITICAL)
required=True,
default=res_cons.SEVERITY_WARNING)
"""Mapped severity of the message"""

12
sushy/tests/unit/resources/registry/test_message_registry.py
View File

@ -63,7 +63,7 @@ class MessageRegistryTestCase(base.TestCase):
self.assertEqual('Panic', self.registry.messages['Failed'].resolution)
self.assertEqual(
2, len(self.registry.messages['MissingThings'].param_types))
self.assertEqual(res_cons.SEVERITY_CRITICAL,
self.assertEqual(res_cons.SEVERITY_WARNING,
self.registry.messages['MissingThings'].severity)
self.assertEqual(
res_cons.PARAMTYPE_STRING,
@ -74,6 +74,16 @@ class MessageRegistryTestCase(base.TestCase):
self.assertEqual(
'Try Later', self.registry.messages['MissingThings'].resolution)
def test__parse_attributes_missing_msg_desc(self):
self.json_doc['Messages']['Success'].pop('Description')
self.registry._parse_attributes(self.json_doc)
self.assertEqual('', self.registry.messages['Success'].description)
def test__parse_attributes_missing_msg_severity(self):
self.json_doc['Messages']['Success'].pop('Severity')
self.registry._parse_attributes(self.json_doc)
self.assertEqual('warning', self.registry.messages['Success'].severity)
def test__parse_attribtues_unknown_param_type(self):
self.registry.json['Messages']['Failed']['ParamTypes'] = \
['unknown_type']