From 23cddf7159ef07103310cc61e732e0493c239b15 Mon Sep 17 00:00:00 2001 From: LingxianKong Date: Tue, 21 Apr 2015 21:10:40 +0800 Subject: [PATCH] Consider input default values in ad-hoc action If the ad-hoc action input default value is defined in the spec, the input param does not needed to be provided for action execution. * rename validate_workflow_input function to validate_input for common purpose of both workflow/action input validation. * delete WorkflowInputException definition, use InputException instead. * add unit test for ad-hoc action execution. Partially Implements: blueprint mistral-default-input-values Change-Id: I3e25c1ce9f7f93dfce3b5981221cf752603ec49e --- mistral/engine/default_engine.py | 2 +- mistral/engine/task_handler.py | 2 + mistral/engine/utils.py | 14 ++--- mistral/exceptions.py | 4 -- .../tests/unit/engine/test_adhoc_actions.py | 56 ++++++++++++++++++- .../tests/unit/engine/test_default_engine.py | 8 +-- 6 files changed, 69 insertions(+), 17 deletions(-) diff --git a/mistral/engine/default_engine.py b/mistral/engine/default_engine.py index bf0ba353..020917c6 100644 --- a/mistral/engine/default_engine.py +++ b/mistral/engine/default_engine.py @@ -55,7 +55,7 @@ class DefaultEngine(base.Engine): wf_def = db_api.get_workflow_definition(wf_name) wf_spec = spec_parser.get_workflow_spec(wf_def.spec) - eng_utils.validate_workflow_input(wf_def, wf_spec, wf_input) + eng_utils.validate_input(wf_def, wf_spec, wf_input) wf_ex = self._create_workflow_execution( wf_def, diff --git a/mistral/engine/task_handler.py b/mistral/engine/task_handler.py index 58629711..333a0afb 100644 --- a/mistral/engine/task_handler.py +++ b/mistral/engine/task_handler.py @@ -332,6 +332,8 @@ def _get_action_input(wf_spec, task_ex, task_spec, ctx): base_name ) + e_utils.validate_input(action_def, action_spec, input_dict) + base_input = action_spec.get_base_input() if base_input: diff --git a/mistral/engine/utils.py b/mistral/engine/utils.py index 252bd5f1..e7888adc 100644 --- a/mistral/engine/utils.py +++ b/mistral/engine/utils.py @@ -27,19 +27,19 @@ from mistral.workflow import utils as wf_utils LOG = logging.getLogger(__name__) -def validate_workflow_input(wf_def, wf_spec, wf_input): - input_param_names = copy.copy((wf_input or {}).keys()) +def validate_input(definition, spec, input): + input_param_names = copy.copy((input or {}).keys()) missing_param_names = [] - for p_name, p_value in six.iteritems(wf_spec.get_input()): + for p_name, p_value in six.iteritems(spec.get_input()): if p_value is utils.NotDefined and p_name not in input_param_names: missing_param_names.append(p_name) if p_name in input_param_names: input_param_names.remove(p_name) if missing_param_names or input_param_names: - msg = 'Invalid workflow input [workflow=%s' - msg_props = [wf_def.name] + msg = 'Invalid input [name=%s, class=%s' + msg_props = [definition.name, spec.__class__.__name__] if missing_param_names: msg += ', missing=%s' @@ -51,11 +51,11 @@ def validate_workflow_input(wf_def, wf_spec, wf_input): msg += ']' - raise exc.WorkflowInputException( + raise exc.InputException( msg % tuple(msg_props) ) else: - utils.merge_dicts(wf_input, wf_spec.get_input(), overwrite=False) + utils.merge_dicts(input, spec.get_input(), overwrite=False) def resolve_action_definition(wf_name, wf_spec_name, action_spec_name): diff --git a/mistral/exceptions.py b/mistral/exceptions.py index d3c19249..0dc7ddf1 100644 --- a/mistral/exceptions.py +++ b/mistral/exceptions.py @@ -84,10 +84,6 @@ class WorkflowException(MistralException): http_code = 400 -class WorkflowInputException(MistralException): - http_code = 400 - - class InputException(MistralException): http_code = 400 diff --git a/mistral/tests/unit/engine/test_adhoc_actions.py b/mistral/tests/unit/engine/test_adhoc_actions.py index 567b0933..350144bd 100644 --- a/mistral/tests/unit/engine/test_adhoc_actions.py +++ b/mistral/tests/unit/engine/test_adhoc_actions.py @@ -15,6 +15,7 @@ from oslo.config import cfg from mistral.db.v2 import api as db_api +from mistral import exceptions as exc from mistral.openstack.common import log as logging from mistral.services import workbooks as wb_service from mistral.tests.unit.engine import base @@ -38,7 +39,7 @@ actions: base-input: output: "<% $.s1 %>+<% $.s2 %>" input: - - s1 + - s1: "a" - s2 output: "<% $ %> and <% $ %>" @@ -57,6 +58,31 @@ workflows: action: concat_twice s1=<% $.str1 %> s2=<% $.str2 %> publish: result: <% $.concat %> + + wf2: + type: direct + input: + - str1 + - str2 + output: + workflow_result: <% $.result %> # Access to execution context variables + concat_task_result: <% $.concat %> # Access to the same but via task name + + tasks: + concat: + action: concat_twice s2=<% $.str2 %> + publish: + result: <% $.concat %> + + wf3: + type: direct + input: + - str1 + - str2 + + tasks: + concat: + action: concat_twice """ @@ -85,3 +111,31 @@ class AdhocActionsTest(base.EngineTestCase): }, wf_ex.output ) + + def test_run_adhoc_action_without_input_value(self): + wf_ex = self.engine.start_workflow( + 'my_wb.wf2', + {'str1': 'a', 'str2': 'b'} + ) + + self._await(lambda: self.is_execution_success(wf_ex.id)) + + wf_ex = db_api.get_workflow_execution(wf_ex.id) + + self.maxDiff = None + + self.assertDictEqual( + { + 'workflow_result': 'a+b and a+b', + 'concat_task_result': 'a+b and a+b' + }, + wf_ex.output + ) + + def test_run_adhoc_action_without_sufficient_input_value(self): + self.assertRaises( + exc.InputException, + self.engine.start_workflow, + 'my_wb.wf3', + {'str1': 'a', 'str2': 'b'} + ) diff --git a/mistral/tests/unit/engine/test_default_engine.py b/mistral/tests/unit/engine/test_default_engine.py index 9d124837..d455af0b 100644 --- a/mistral/tests/unit/engine/test_default_engine.py +++ b/mistral/tests/unit/engine/test_default_engine.py @@ -250,7 +250,7 @@ class DefaultEngineTest(base.DbTestCase): def test_start_workflow_missing_parameters(self): self.assertRaises( - exc.WorkflowInputException, + exc.InputException, self.engine.start_workflow, 'wb.wf', None, @@ -259,7 +259,7 @@ class DefaultEngineTest(base.DbTestCase): def test_start_workflow_unexpected_parameters(self): self.assertRaises( - exc.WorkflowInputException, + exc.InputException, self.engine.start_workflow, 'wb.wf', {'param1': 'Hey', 'param2': 'Hi', 'unexpected_param': 'val'}, @@ -428,12 +428,12 @@ class DefaultEngineWithTransportTest(eng_test_base.EngineTestCase): def test_engine_client_remote_error(self): mocked = mock.Mock() mocked.call.side_effect = rpc_client.RemoteError( - 'WorkflowInputException', + 'InputException', 'Input is wrong' ) self.engine_client._client = mocked self.assertRaises( - exc.WorkflowInputException, + exc.InputException, self.engine_client.start_workflow, 'some_wf', {} )