From 2b15c8d0b522aca34fd41c950861766423a31c93 Mon Sep 17 00:00:00 2001 From: Dmitri Zimine Date: Thu, 19 Jun 2014 16:34:37 -0700 Subject: [PATCH] 'requires' should take a string or list * 'requires' can be a task name, or a list of task names. No use case for a dictionary. * added convenience methods to get requires. * minor fix with TaskSpec: get_dict... returns {} not None. Change-Id: I1c9ef4ab41679e10d8aab38dfae68f169608bd22 --- mistral/db/sqlalchemy/models.py | 2 +- mistral/engine/__init__.py | 2 +- mistral/engine/workflow.py | 35 +++++++++---------- .../resources/control_flow/require_flow.yaml | 5 +-- .../tests/unit/db/test_sqlalchemy_db_api.py | 4 +-- .../unit/engine/default/test_executor.py | 2 +- mistral/tests/unit/engine/test_workflow.py | 2 +- mistral/workbook/tasks.py | 13 ++++--- 8 files changed, 33 insertions(+), 32 deletions(-) diff --git a/mistral/db/sqlalchemy/models.py b/mistral/db/sqlalchemy/models.py index d93824b3..b25b45c4 100644 --- a/mistral/db/sqlalchemy/models.py +++ b/mistral/db/sqlalchemy/models.py @@ -87,7 +87,7 @@ class Task(mb.MistralBase): id = _id_column() name = sa.Column(sa.String(80)) - requires = sa.Column(st.JsonDictType()) + requires = sa.Column(st.JsonListType()) workbook_name = sa.Column(sa.String(80)) execution_id = sa.Column(sa.String(36)) description = sa.Column(sa.String(200)) diff --git a/mistral/engine/__init__.py b/mistral/engine/__init__.py index 71868c00..5b8c28c5 100644 --- a/mistral/engine/__init__.py +++ b/mistral/engine/__init__.py @@ -303,7 +303,7 @@ class Engine(object): db_task = db_api.task_create(workbook_name, execution_id, { "name": task.name, - "requires": task.requires, + "requires": task.get_requires(), "task_spec": task.to_dict(), "action_spec": {} if not action_spec else action_spec.to_dict(), diff --git a/mistral/engine/workflow.py b/mistral/engine/workflow.py index f7e4d499..de44dc08 100644 --- a/mistral/engine/workflow.py +++ b/mistral/engine/workflow.py @@ -42,7 +42,22 @@ def find_workflow_tasks(workbook, task_name): def find_resolved_tasks(tasks): # We need to analyse graph and see which tasks are ready to start - return _get_resolved_tasks(tasks) + resolved_tasks = [] + delayed_tasks = [] + allows = [] + for t in tasks: + if t['state'] == states.SUCCESS: + allows += [t['name']] + allow_set = set(allows) + for t in tasks: + deps = t.get('requires', []) + if len(set(deps) - allow_set) == 0: + # all required tasks, if any, are SUCCESS + if t['state'] == states.IDLE: + resolved_tasks.append(t) + elif t['state'] == states.DELAYED: + delayed_tasks.append(t) + return resolved_tasks, delayed_tasks def _get_checked_tasks(target_tasks): @@ -134,21 +149,3 @@ def _update_dependencies(tasks, graph): for task in tasks: for dep in _get_dependency_tasks(tasks, task): graph.add_edge(dep, task) - - -def _get_resolved_tasks(tasks): - resolved_tasks = [] - delayed_tasks = [] - allows = [] - for t in tasks: - if t['state'] == states.SUCCESS: - allows += [t['name']] - allow_set = set(allows) - for t in tasks: - deps = t.get('requires', {}).keys() - if len(set(deps) - allow_set) == 0: - if t['state'] == states.IDLE: - resolved_tasks.append(t) - elif t['state'] == states.DELAYED: - delayed_tasks.append(t) - return resolved_tasks, delayed_tasks diff --git a/mistral/tests/resources/control_flow/require_flow.yaml b/mistral/tests/resources/control_flow/require_flow.yaml index 733abb24..4538d89b 100644 --- a/mistral/tests/resources/control_flow/require_flow.yaml +++ b/mistral/tests/resources/control_flow/require_flow.yaml @@ -45,7 +45,7 @@ Workflow: flavor_id: 42 attach-volumes: - requires: attach-volumes + requires: create-vms action: MyRest.attach-volume parameters: size: 1234 @@ -58,7 +58,8 @@ Workflow: server_id: 123 backup-vms: - requires: [create-vms] + requires: + - create-vms action: MyRest.backup-vm parameters: server_id: 123 diff --git a/mistral/tests/unit/db/test_sqlalchemy_db_api.py b/mistral/tests/unit/db/test_sqlalchemy_db_api.py index 197e5668..c593225e 100644 --- a/mistral/tests/unit/db/test_sqlalchemy_db_api.py +++ b/mistral/tests/unit/db/test_sqlalchemy_db_api.py @@ -232,7 +232,7 @@ TASKS = [ 'execution_id': '1', 'name': 'my_task1', 'description': 'my description', - 'requires': {'my_task2': '', 'my_task3': ''}, + 'requires': ['my_task2', 'my_task3'], 'task_spec': None, 'action_spec': None, 'action': {'name': 'Nova:create-vm'}, @@ -250,7 +250,7 @@ TASKS = [ 'execution_id': '1', 'name': 'my_task2', 'description': 'my description', - 'requires': {'my_task4': '', 'my_task5': ''}, + 'requires': ['my_task4', 'my_task5'], 'task_spec': None, 'action_spec': None, 'action': {'name': 'Cinder:create-volume'}, diff --git a/mistral/tests/unit/engine/default/test_executor.py b/mistral/tests/unit/engine/default/test_executor.py index 7de05fe9..51362894 100644 --- a/mistral/tests/unit/engine/default/test_executor.py +++ b/mistral/tests/unit/engine/default/test_executor.py @@ -79,7 +79,7 @@ SAMPLE_TASK = { 'task_spec': { 'action': 'MyRest.my-action', 'name': TASK_NAME}, - 'requires': {}, + 'requires': [], 'state': states.IDLE} SAMPLE_CONTEXT = { diff --git a/mistral/tests/unit/engine/test_workflow.py b/mistral/tests/unit/engine/test_workflow.py index c23dfb90..cbe4a31a 100644 --- a/mistral/tests/unit/engine/test_workflow.py +++ b/mistral/tests/unit/engine/test_workflow.py @@ -31,7 +31,7 @@ TASKS = [ 'state': states.SUCCESS }, { - 'requires': {'create-vms': ''}, + 'requires': ['create-vms'], 'name': 'attach-volume', 'state': states.IDLE } diff --git a/mistral/workbook/tasks.py b/mistral/workbook/tasks.py index b865e80e..38f7a08f 100644 --- a/mistral/workbook/tasks.py +++ b/mistral/workbook/tasks.py @@ -42,11 +42,11 @@ class TaskSpec(base.BaseSpec): elif isinstance(req, dict): task['requires'] = req - def _get_on_state(self, key): + def _get_as_dict(self, key): tasks = self.get_property(key) if not tasks: - return None + return {} if isinstance(tasks, dict): return tasks @@ -61,14 +61,17 @@ class TaskSpec(base.BaseSpec): def get_property(self, property_name, default=None): return self._data.get(property_name, default) + def get_requires(self): + return self._get_as_dict('requires').keys() + def get_on_error(self): - return self._get_on_state("on-error") + return self._get_as_dict("on-error") def get_on_success(self): - return self._get_on_state("on-success") + return self._get_as_dict("on-success") def get_on_finish(self): - return self._get_on_state("on-finish") + return self._get_as_dict("on-finish") def get_action_namespace(self): return self.action.split('.')[0]