openstack/mistral
Code Issues Proposed changes

Remove extra a specification validation

Browse Source 
Currently when we get a specification using the instantiate_spec function,
we always validate their schema and semantics over and over again.
To prevent it we add new validate parameter to a Spec class.
The validate parameter must be True when we create a workflow, workbook
or action using a mistral-api. In all other cases, it must be False.

Change-Id: Ia450ea9635bc75c204fe031cfeeab154f1d03862
Closes-Bug: #1738769
Signed-off-by: Vitalii Solodilov <mcdkr@yandex.ru>
This commit is contained in:
Vitalii Solodilov 2018-07-29 18:17:51 +04:00
parent b01d6c6642
commit 4bf03d8e7d
14 changed files with 90 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
mistral/api/controllers/v2/validation.py
View File

@ -32,7 +32,7 @@ class SpecValidationController(rest.RestController):
definition = pecan.request.text
try:
self._parse_func(definition)
self._parse_func(definition, validate=True)
except exc.DSLParsingException as e:
return {'valid': False, 'error': str(e)}

47
mistral/lang/base.py
View File

@ -52,7 +52,7 @@ ALL = (
PARAMS_PTRN = re.compile("([-_\w]+)=(%s)" % "|".join(ALL))
def instantiate_spec(spec_cls, data):
def instantiate_spec(spec_cls, data, validate=False):
"""Instantiates specification accounting for specification hierarchies.
:param spec_cls: Specification concrete or base class. In case if base
@ -60,17 +60,22 @@ def instantiate_spec(spec_cls, data):
_polymorphic_key and _polymorphic_value in order to find a concrete
class that needs to be instantiated.
:param data: Raw specification data as a dictionary.
:type data: dict
:param validate: If it equals False then semantics and schema validation
will be skipped
:type validate: bool
"""
if issubclass(spec_cls, BaseSpecList):
# Ignore polymorphic search for specification lists because
# it doesn't make sense for them.
return spec_cls(data)
return spec_cls(data, validate)
if not hasattr(spec_cls, '_polymorphic_key'):
spec = spec_cls(data)
spec = spec_cls(data, validate)
spec.validate_semantics()
if validate:
spec.validate_semantics()
return spec
@ -101,9 +106,10 @@ def instantiate_spec(spec_cls, data):
)
if cls._polymorphic_value == data.get(key_name, key_default):
spec = cls(data)
spec = cls(data, validate)
spec.validate_semantics()
if validate:
spec.validate_semantics()
return spec
@ -176,10 +182,12 @@ class BaseSpec(object):
return schema
def __init__(self, data):
def __init__(self, data, validate):
self._data = data
self._validate = validate
self.validate_schema()
if validate:
self.validate_schema()
def validate_schema(self):
"""Validates DSL entity schema that this specification represents.
@ -227,10 +235,10 @@ class BaseSpec(object):
def _spec_property(self, prop_name, spec_cls):
prop_val = self._data.get(prop_name)
return (
instantiate_spec(spec_cls, prop_val) if prop_val is not None
else None
)
if prop_val is not None:
return instantiate_spec(spec_cls, prop_val, self._validate)
else:
return None
def _group_spec(self, spec_cls, *prop_names):
if not prop_names:
@ -244,7 +252,7 @@ class BaseSpec(object):
if prop_val:
data[prop_name] = prop_val
return instantiate_spec(spec_cls, data)
return instantiate_spec(spec_cls, data, self._validate)
def _inject_version(self, prop_names):
for prop_name in prop_names:
@ -322,8 +330,8 @@ class BaseListSpec(BaseSpec):
"required": ["version"],
}
def __init__(self, data):
super(BaseListSpec, self).__init__(data)
def __init__(self, data, validate):
super(BaseListSpec, self).__init__(data, validate)
self.items = []
@ -331,7 +339,10 @@ class BaseListSpec(BaseSpec):
if k != 'version':
v['name'] = k
self._inject_version([k])
self.items.append(instantiate_spec(self.item_class, v))
self.items.append(
instantiate_spec(self.item_class, v, validate)
)
def validate_schema(self):
super(BaseListSpec, self).validate_schema()
@ -357,7 +368,7 @@ class BaseSpecList(object):
_version = '2.0'
def __init__(self, data):
def __init__(self, data, validate):
self.items = {}
for k, v in data.items():
@ -369,7 +380,7 @@ class BaseSpecList(object):
v['name'] = k
v['version'] = self._version
self.items[k] = instantiate_spec(self.item_class, v)
self.items[k] = instantiate_spec(self.item_class, v, validate)
def item_keys(self):
return self.items.keys()

30
mistral/lang/parser.py
View File

@ -80,15 +80,17 @@ def _get_spec_version(spec_dict):
# Factory methods to get specifications either from raw YAML formatted text or
# from dictionaries parsed from YAML formatted text.
def get_workbook_spec(spec_dict):
def get_workbook_spec(spec_dict, validate):
if _get_spec_version(spec_dict) == V2_0:
return base.instantiate_spec(wb_v2.WorkbookSpec, spec_dict)
return base.instantiate_spec(
wb_v2.WorkbookSpec, spec_dict, validate
)
return None
def get_workbook_spec_from_yaml(text):
return get_workbook_spec(parse_yaml(text))
def get_workbook_spec_from_yaml(text, validate=True):
return get_workbook_spec(parse_yaml(text), validate)
def get_action_spec(spec_dict):
@ -106,12 +108,14 @@ def get_action_spec_from_yaml(text, action_name):
return get_action_spec(spec_dict)
def get_action_list_spec(spec_dict):
return base.instantiate_spec(actions_v2.ActionListSpec, spec_dict)
def get_action_list_spec(spec_dict, validate):
return base.instantiate_spec(
actions_v2.ActionListSpec, spec_dict, validate
)
def get_action_list_spec_from_yaml(text):
return get_action_list_spec(parse_yaml(text))
def get_action_list_spec_from_yaml(text, validate=True):
return get_action_list_spec(parse_yaml(text), validate=validate)
def get_workflow_spec(spec_dict):
@ -130,16 +134,18 @@ def get_workflow_spec(spec_dict):
return None
def get_workflow_list_spec(spec_dict):
return base.instantiate_spec(wf_v2.WorkflowListSpec, spec_dict)
def get_workflow_list_spec(spec_dict, validate):
return base.instantiate_spec(
wf_v2.WorkflowListSpec, spec_dict, validate
)
def get_workflow_spec_from_yaml(text):
return get_workflow_spec(parse_yaml(text))
def get_workflow_list_spec_from_yaml(text):
return get_workflow_list_spec(parse_yaml(text))
def get_workflow_list_spec_from_yaml(text, validate=True):
return get_workflow_list_spec(parse_yaml(text), validate)
def get_task_spec(spec_dict):

4
mistral/lang/v2/actions.py
View File

@ -34,8 +34,8 @@ class ActionSpec(base.BaseSpec):
"additionalProperties": False
}
def __init__(self, data):
super(ActionSpec, self).__init__(data)
def __init__(self, data, validate):
super(ActionSpec, self).__init__(data, validate)
self._name = data['name']
self._description = data.get('description')

4
mistral/lang/v2/on_clause.py
View File

@ -94,8 +94,8 @@ class OnClauseSpec(base.BaseSpec):
]
}
def __init__(self, data):
super(OnClauseSpec, self).__init__(data)
def __init__(self, data, validate):
super(OnClauseSpec, self).__init__(data, validate)
if not isinstance(data, dict):
# Old simple schema.

4
mistral/lang/v2/policies.py
View File

@ -37,8 +37,8 @@ class PoliciesSpec(base.BaseSpec):
def get_schema(cls, includes=['definitions']):
return super(PoliciesSpec, cls).get_schema(includes)
def __init__(self, data):
super(PoliciesSpec, self).__init__(data)
def __init__(self, data, validate):
super(PoliciesSpec, self).__init__(data, validate)
self._retry = self._spec_property('retry', retry_policy.RetrySpec)
self._wait_before = data.get('wait-before', 0)

4
mistral/lang/v2/publish.py
View File

@ -29,8 +29,8 @@ class PublishSpec(base.BaseSpec):
"additionalProperties": False
}
def __init__(self, data):
super(PublishSpec, self).__init__(data)
def __init__(self, data, validate):
super(PublishSpec, self).__init__(data, validate)
self._branch = self._data.get('branch')
self._global = self._data.get('global')

4
mistral/lang/v2/retry_policy.py
View File

@ -54,10 +54,10 @@ class RetrySpec(base.BaseSpec):
def get_schema(cls, includes=['definitions']):
return super(RetrySpec, cls).get_schema(includes)
def __init__(self, data):
def __init__(self, data, validate):
data = self._transform_retry_one_line(data)
super(RetrySpec, self).__init__(data)
super(RetrySpec, self).__init__(data, validate)
self._break_on = data.get('break-on')
self._count = data.get('count')

4
mistral/lang/v2/task_defaults.py
View File

@ -53,8 +53,8 @@ class TaskDefaultsSpec(base.BaseSpec):
def get_schema(cls, includes=['definitions']):
return super(TaskDefaultsSpec, cls).get_schema(includes)
def __init__(self, data):
super(TaskDefaultsSpec, self).__init__(data)
def __init__(self, data, validate):
super(TaskDefaultsSpec, self).__init__(data, validate)
self._policies = self._group_spec(
policies.PoliciesSpec,

20
mistral/lang/v2/tasks.py
View File

@ -107,8 +107,8 @@ class TaskSpec(base.BaseSpec):
]
}
def __init__(self, data):
super(TaskSpec, self).__init__(data)
def __init__(self, data, validate):
super(TaskSpec, self).__init__(data, validate)
self._name = data['name']
self._description = data.get('description')
@ -248,10 +248,14 @@ class TaskSpec(base.BaseSpec):
spec = None
if state == states.SUCCESS and self._publish:
spec = publish.PublishSpec({'branch': self._publish})
spec = publish.PublishSpec(
{'branch': self._publish},
validate=self._validate
)
elif state == states.ERROR and self._publish_on_error:
spec = publish.PublishSpec(
{'branch': self._publish_on_error}
{'branch': self._publish_on_error},
validate=self._validate
)
return spec
@ -291,8 +295,8 @@ class DirectWorkflowTaskSpec(TaskSpec):
_direct_workflow_schema
)
def __init__(self, data):
super(DirectWorkflowTaskSpec, self).__init__(data)
def __init__(self, data, validate):
super(DirectWorkflowTaskSpec, self).__init__(data, validate)
self._join = data.get('join')
@ -376,8 +380,8 @@ class ReverseWorkflowTaskSpec(TaskSpec):
_reverse_workflow_schema
)
def __init__(self, data):
super(ReverseWorkflowTaskSpec, self).__init__(data)
def __init__(self, data, validate):
super(ReverseWorkflowTaskSpec, self).__init__(data, validate)
self._requires = data.get('requires', [])

4
mistral/lang/v2/workbook.py
View File

@ -51,8 +51,8 @@ class WorkbookSpec(base.BaseSpec):
"additionalProperties": False
}
def __init__(self, data):
super(WorkbookSpec, self).__init__(data)
def __init__(self, data, validate):
super(WorkbookSpec, self).__init__(data, validate)
self._inject_version(['actions', 'workflows'])

8
mistral/lang/v2/workflows.py
View File

@ -44,8 +44,8 @@ class WorkflowSpec(base.BaseSpec):
"additionalProperties": False
}
def __init__(self, data):
super(WorkflowSpec, self).__init__(data)
def __init__(self, data, validate):
super(WorkflowSpec, self).__init__(data, validate)
self._name = data['name']
self._description = data.get('description')
@ -152,8 +152,8 @@ class DirectWorkflowSpec(WorkflowSpec):
}
}
def __init__(self, data):
super(DirectWorkflowSpec, self).__init__(data)
def __init__(self, data, validate):
super(DirectWorkflowSpec, self).__init__(data, validate)
# Init simple dictionary based caches for inbound and
# outbound task specifications. In fact, we don't need

8
mistral/services/workbooks.py
View File

@ -18,7 +18,9 @@ from mistral.services import actions
def create_workbook_v2(definition, namespace='', scope='private'):
wb_spec = spec_parser.get_workbook_spec_from_yaml(definition)
wb_spec = spec_parser.get_workbook_spec_from_yaml(
definition, validate=True
)
wb_values = _get_workbook_values(
wb_spec,
@ -36,7 +38,9 @@ def create_workbook_v2(definition, namespace='', scope='private'):
def update_workbook_v2(definition, namespace='', scope='private'):
wb_spec = spec_parser.get_workbook_spec_from_yaml(definition)
wb_spec = spec_parser.get_workbook_spec_from_yaml(
definition, validate=True
)
values = _get_workbook_values(wb_spec, definition, scope, namespace)

8
mistral/services/workflows.py
View File

@ -54,7 +54,9 @@ def sync_db():
def create_workflows(definition, scope='private', is_system=False,
run_in_tx=True, namespace=''):
LOG.debug("Creating workflows")
wf_list_spec = spec_parser.get_workflow_list_spec_from_yaml(definition)
wf_list_spec = spec_parser.get_workflow_list_spec_from_yaml(
definition, validate=True
)
db_wfs = []
if run_in_tx:
@ -98,7 +100,9 @@ def update_workflows(definition, scope='private', identifier=None,
namespace=''):
LOG.debug("Updating workflows")
wf_list_spec = spec_parser.get_workflow_list_spec_from_yaml(definition)
wf_list_spec = spec_parser.get_workflow_list_spec_from_yaml(
definition, validate=True
)
wfs = wf_list_spec.get_workflows()
if identifier and len(wfs) > 1: