From 861d572b2357cfa9f32f97e53c4bae76ba231cfb Mon Sep 17 00:00:00 2001 From: Michal Gershenzon Date: Tue, 14 Feb 2017 17:52:16 +0000 Subject: [PATCH] Remove output from list action executions API output field should be returned only when getting one action execution Change-Id: I9913f4893b3eaf30faca1a747eeeda493dbc0fb2 Closes-Bug: #1610817 --- .../api/controllers/v2/action_execution.py | 33 +++++++--- .../unit/api/v2/test_action_executions.py | 33 ++++++++-- .../services/v2/mistral_client.py | 11 +++- .../tests/api/v2/test_action_executions.py | 65 +++++++++++++++++++ .../tests/api/v2/test_tasks.py | 12 ++++ ...ction-execution-list-c946f1b38dc5a052.yaml | 16 +++++ 6 files changed, 154 insertions(+), 16 deletions(-) create mode 100644 releasenotes/notes/include-output-paramter-in-action-execution-list-c946f1b38dc5a052.yaml diff --git a/mistral/api/controllers/v2/action_execution.py b/mistral/api/controllers/v2/action_execution.py index 4dc3464cd..db21a2635 100644 --- a/mistral/api/controllers/v2/action_execution.py +++ b/mistral/api/controllers/v2/action_execution.py @@ -43,14 +43,17 @@ def _load_deferred_output_field(action_ex): def _get_action_execution(id): with db_api.transaction(): - action_ex = db_api.get_action_execution(id) - - return _get_action_execution_resource(action_ex) + return _get_action_execution_resource(db_api.get_action_execution(id)) def _get_action_execution_resource(action_ex): _load_deferred_output_field(action_ex) + return _get_action_execution_resource_for_list(action_ex) + + +def _get_action_execution_resource_for_list(action_ex): + # TODO(nmakhotkin): Get rid of using dicts for constructing resources. # TODO(nmakhotkin): Use db_model for this instead. res = resources.ActionExecution.from_dict(action_ex.to_dict()) @@ -64,7 +67,7 @@ def _get_action_execution_resource(action_ex): def _get_action_executions(task_execution_id=None, marker=None, limit=None, sort_keys='created_at', sort_dirs='asc', - fields='', **filters): + fields='', include_output=False, **filters): """Return all action executions. Where project_id is the same as the requester or @@ -89,12 +92,17 @@ def _get_action_executions(task_execution_id=None, marker=None, limit=None, if task_execution_id: filters['task_execution_id'] = task_execution_id + if include_output: + resource_function = _get_action_execution_resource + else: + resource_function = _get_action_execution_resource_for_list + return rest_utils.get_all( resources.ActionExecutions, resources.ActionExecution, db_api.get_action_executions, db_api.get_action_execution, - resource_function=_get_action_execution_resource, + resource_function=resource_function, marker=marker, limit=limit, sort_keys=sort_keys, @@ -186,13 +194,13 @@ class ActionExecutionsController(rest.RestController): wtypes.text, wtypes.text, wtypes.text, wtypes.text, wtypes.text, wtypes.text, types.uuid, wtypes.text, wtypes.text, bool, types.jsontype, - types.jsontype, types.jsontype, wtypes.text) + types.jsontype, types.jsontype, wtypes.text, bool) def get_all(self, marker=None, limit=None, sort_keys='created_at', sort_dirs='asc', fields='', created_at=None, name=None, tags=None, updated_at=None, workflow_name=None, task_name=None, task_execution_id=None, state=None, state_info=None, accepted=None, input=None, output=None, - params=None, description=None): + params=None, description=None, include_output=False): """Return all tasks within the execution. Where project_id is the same as the requester or @@ -234,6 +242,8 @@ class ActionExecutionsController(rest.RestController): time and date. :param updated_at: Optional. Keep only resources with specific latest update time and date. + :param include_output: Optional. Include the output for all executions + in the list """ acl.enforce('action_executions:list', context.ctx()) @@ -264,6 +274,7 @@ class ActionExecutionsController(rest.RestController): sort_keys=sort_keys, sort_dirs=sort_dirs, fields=fields, + include_output=include_output, **filters ) @@ -302,13 +313,14 @@ class TasksActionExecutionController(rest.RestController): wtypes.text, types.uniquelist, wtypes.text, wtypes.text, wtypes.text, wtypes.text, wtypes.text, wtypes.text, bool, types.jsontype, types.jsontype, - types.jsontype, wtypes.text) + types.jsontype, wtypes.text, bool) def get_all(self, task_execution_id, marker=None, limit=None, sort_keys='created_at', sort_dirs='asc', fields='', created_at=None, name=None, tags=None, updated_at=None, workflow_name=None, task_name=None, state=None, state_info=None, accepted=None, input=None, - output=None, params=None, description=None): + output=None, params=None, description=None, + include_output=None): """Return all tasks within the execution. Where project_id is the same as the requester or @@ -350,6 +362,8 @@ class TasksActionExecutionController(rest.RestController): time and date. :param updated_at: Optional. Keep only resources with specific latest update time and date. + :param include_output: Optional. Include the output for all executions + in the list """ acl.enforce('action_executions:list', context.ctx()) @@ -380,6 +394,7 @@ class TasksActionExecutionController(rest.RestController): sort_keys=sort_keys, sort_dirs=sort_dirs, fields=fields, + include_output=include_output, **filters ) diff --git a/mistral/tests/unit/api/v2/test_action_executions.py b/mistral/tests/unit/api/v2/test_action_executions.py index e448b7924..2fef8f5e1 100644 --- a/mistral/tests/unit/api/v2/test_action_executions.py +++ b/mistral/tests/unit/api/v2/test_action_executions.py @@ -22,11 +22,13 @@ import mock from oslo_config import cfg import oslo_messaging +from mistral.api.controllers.v2 import action_execution from mistral.db.v2 import api as db_api from mistral.db.v2.sqlalchemy import models from mistral.engine.rpc_backend import rpc from mistral import exceptions as exc from mistral.tests.unit.api import base +from mistral.utils import rest_utils from mistral.workflow import states from mistral.workflow import utils as wf_utils @@ -448,6 +450,29 @@ class TestActionExecutionsController(base.APITest): self.assertEqual(1, len(resp.json['action_executions'])) self.assertDictEqual(ACTION_EX, resp.json['action_executions'][0]) + @mock.patch.object(db_api, 'get_action_executions', MOCK_ACTIONS) + @mock.patch.object(rest_utils, 'get_all') + def test_get_all_with_and_without_output(self, mock_get_all): + resp = self.app.get('/v2/action_executions') + args, kwargs = mock_get_all.call_args + resource_function = kwargs['resource_function'] + + self.assertEqual(200, resp.status_int) + self.assertEqual( + action_execution._get_action_execution_resource_for_list, + resource_function + ) + + resp = self.app.get('/v2/action_executions?include_output=true') + args, kwargs = mock_get_all.call_args + resource_function = kwargs['resource_function'] + + self.assertEqual(200, resp.status_int) + self.assertEqual( + action_execution._get_action_execution_resource, + resource_function + ) + @mock.patch.object(db_api, 'get_action_executions', MOCK_EMPTY) def test_get_all_empty(self): resp = self.app.get('/v2/action_executions') @@ -483,7 +508,7 @@ class TestActionExecutionsController(base.APITest): ) @mock.patch.object(db_api, 'get_action_execution', MOCK_ACTION) - def test_delete_action_exeuction_with_task(self): + def test_delete_action_execution_with_task(self): cfg.CONF.set_default('allow_action_execution_deletion', True, 'api') resp = self.app.delete('/v2/action_executions/123', expect_errors=True) @@ -499,7 +524,7 @@ class TestActionExecutionsController(base.APITest): 'get_action_execution', MOCK_ACTION_NOT_COMPLETE ) - def test_delete_action_exeuction_not_complete(self): + def test_delete_action_execution_not_complete(self): cfg.CONF.set_default('allow_action_execution_deletion', True, 'api') resp = self.app.delete('/v2/action_executions/123', expect_errors=True) @@ -516,7 +541,7 @@ class TestActionExecutionsController(base.APITest): MOCK_ACTION_COMPLETE_ERROR ) @mock.patch.object(db_api, 'delete_action_execution', MOCK_DELETE) - def test_delete_action_exeuction_complete_error(self): + def test_delete_action_execution_complete_error(self): cfg.CONF.set_default('allow_action_execution_deletion', True, 'api') resp = self.app.delete('/v2/action_executions/123', expect_errors=True) @@ -529,7 +554,7 @@ class TestActionExecutionsController(base.APITest): MOCK_ACTION_COMPLETE_CANCELLED ) @mock.patch.object(db_api, 'delete_action_execution', MOCK_DELETE) - def test_delete_action_exeuction_complete_cancelled(self): + def test_delete_action_execution_complete_cancelled(self): cfg.CONF.set_default('allow_action_execution_deletion', True, 'api') resp = self.app.delete('/v2/action_executions/123', expect_errors=True) diff --git a/mistral_tempest_tests/services/v2/mistral_client.py b/mistral_tempest_tests/services/v2/mistral_client.py index 207eff368..f9022caa0 100644 --- a/mistral_tempest_tests/services/v2/mistral_client.py +++ b/mistral_tempest_tests/services/v2/mistral_client.py @@ -75,6 +75,9 @@ class MistralClientV2(base.MistralClientBase): return resp, json.loads(body) + def get_action_execution(self, action_execution_id): + return self.get('action_executions/%s' % action_execution_id) + def create_execution(self, identifier, wf_input=None, params=None): if uuidutils.is_uuid_like(identifier): body = {"workflow_id": "%s" % identifier} @@ -130,9 +133,11 @@ class MistralClientV2(base.MistralClientBase): return [t for t in all_tasks if t['workflow_name'] == wf_name] def create_action_execution(self, request_body, extra_headers={}): - resp, body = self.post_json('action_executions', - request_body, - extra_headers) + resp, body = self.post_json( + 'action_executions', + request_body, + extra_headers + ) params = json.loads(request_body.get('params', '{}')) if params.get('save_result', False): diff --git a/mistral_tempest_tests/tests/api/v2/test_action_executions.py b/mistral_tempest_tests/tests/api/v2/test_action_executions.py index 4065e3266..2d635fc48 100644 --- a/mistral_tempest_tests/tests/api/v2/test_action_executions.py +++ b/mistral_tempest_tests/tests/api/v2/test_action_executions.py @@ -28,6 +28,17 @@ LOG = logging.getLogger(__name__) class ActionExecutionTestsV2(base.TestCase): _service = 'workflowv2' + @classmethod + def resource_setup(cls): + super(ActionExecutionTestsV2, cls).resource_setup() + + cls.client.create_action_execution( + { + 'name': 'std.echo', + 'input': '{"output": "Hello, Mistral!"}' + } + ) + @classmethod def resource_cleanup(cls): for action_ex in cls.client.action_executions: @@ -58,6 +69,60 @@ class ActionExecutionTestsV2(base.TestCase): output ) + @test.attr(type='sanity') + def test_list_action_executions(self): + resp, body = self.client.get_list_obj('action_executions') + + self.assertEqual(200, resp.status) + + @test.attr(type='sanity') + def test_output_appear_in_response_only_when_needed(self): + resp, body = self.client.get_list_obj('action_executions') + + self.assertEqual(200, resp.status) + action_execution = body['action_executions'][0] + self.assertNotIn("output", action_execution) + + resp, body = self.client.get_list_obj( + 'action_executions?include_output=True' + ) + + self.assertEqual(200, resp.status) + action_execution = body['action_executions'][0] + self.assertIn("output", action_execution) + + resp, body = self.client.get_action_execution(action_execution['id']) + self.assertIn("output", body) + + # Test when passing task execution ID + + resp, body = self.client.create_workflow('wf_v2.yaml') + wf_name = body['workflows'][0]['name'] + self.assertEqual(201, resp.status) + resp, body = self.client.create_execution(wf_name) + self.assertEqual(201, resp.status) + resp, body = self.client.get_list_obj('tasks') + self.assertEqual(200, resp.status) + task_id = body['tasks'][0]['id'] + + resp, body = self.client.get_list_obj( + 'action_executions?include_output=true&task_execution_id=%s' % + task_id + ) + + self.assertEqual(200, resp.status) + action_execution = body['action_executions'][0] + self.assertIn("output", action_execution) + + resp, body = self.client.get_list_obj( + 'action_executions?&task_execution_id=%s' % + task_id + ) + + self.assertEqual(200, resp.status) + action_execution = body['action_executions'][0] + self.assertNotIn("output", action_execution) + @test.attr(type='sanity') def test_run_action_std_http(self): resp, body = self.client.create_action_execution( diff --git a/mistral_tempest_tests/tests/api/v2/test_tasks.py b/mistral_tempest_tests/tests/api/v2/test_tasks.py index c4fc5c42b..c71c31cf1 100644 --- a/mistral_tempest_tests/tests/api/v2/test_tasks.py +++ b/mistral_tempest_tests/tests/api/v2/test_tasks.py @@ -28,6 +28,7 @@ class TasksTestsV2(base.TestCase): _, body = self.client.create_workflow('wf_v2.yaml') self.direct_wf_name = body['workflows'][0]['name'] _, execution = self.client.create_execution(self.direct_wf_name) + self.execution_id = execution['id'] def tearDown(self): for wf in self.client.workflows: @@ -56,6 +57,17 @@ class TasksTestsV2(base.TestCase): self.direct_wf_name, body['tasks'][-1]['workflow_name'] ) + @test.attr(type='sanity') + def test_get_tasks_of_execution(self): + resp, body = self.client.get_list_obj( + 'tasks?workflow_execution_id=%s' % self.execution_id + ) + + self.assertEqual(200, resp.status) + self.assertEqual( + self.direct_wf_name, body['tasks'][-1]['workflow_name'] + ) + class TaskTypesTestsV2(base.TestCase): diff --git a/releasenotes/notes/include-output-paramter-in-action-execution-list-c946f1b38dc5a052.yaml b/releasenotes/notes/include-output-paramter-in-action-execution-list-c946f1b38dc5a052.yaml new file mode 100644 index 000000000..24bd2c69c --- /dev/null +++ b/releasenotes/notes/include-output-paramter-in-action-execution-list-c946f1b38dc5a052.yaml @@ -0,0 +1,16 @@ +--- + +features: + - | + + New parameter called 'include_output' added to action execution api. + By default output field does not return when calling list action executions + API + +critical: + - | + + By default, output field will not return when calling list action + executions. In the previous version it did, so if a user used this, and/or + wants to get output field when calling list action executions API, it will + be possible only by using the new include output parameter.