Return the result of the MistralHTTPAction

If the HTTP request fails, we need to fail the task. Returning the error
from the parent class will do this. While this means we also return the
success result it will be ignored by the Mistral engine.

Credit to @lijianying10 on GitHub for sending this fix via a pull
request. Tests were then added to verify the change.

Closes-Bug: #1779973
Change-Id: Ib8754c8de2d6677d71383b3793d0fa168be575f5
This commit is contained in:
Dougal Matthews 2018-07-09 16:17:39 +01:00
parent 9b55db4bf0
commit 549ec1f3bf
4 changed files with 184 additions and 5 deletions

View File

@ -272,7 +272,7 @@ class MistralHTTPAction(HTTPAction):
'Mistral-Callback-URL': exec_ctx.callback_url,
})
super(MistralHTTPAction, self).run(context)
return super(MistralHTTPAction, self).run(context)
def is_sync(self):
return False

View File

@ -0,0 +1,6 @@
---
features:
- |
The action std.mistral_http will now retrun an error if the HTTP request
fails. Previously the task would still go into the RUNNING state and wait
to be completed by the external resource.

View File

@ -0,0 +1,124 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import json
import mock
import requests
from mistral.actions import std_actions as std
from mistral.tests.unit import base
from mistral_lib import actions as mistral_lib_actions
URL = 'http://some_url'
DATA = {
'server': {
'id': '12345',
'metadata': {
'name': 'super_server'
}
}
}
def get_fake_response(content, code, **kwargs):
return base.FakeHTTPResponse(
content,
code,
**kwargs
)
def get_success_fake_response():
return get_fake_response(
json.dumps(DATA),
200,
headers={'Content-Type': 'application/json'}
)
def get_error_fake_response():
return get_fake_response(
json.dumps(DATA),
401
)
class MistralHTTPActionTest(base.BaseTest):
@mock.patch.object(requests, 'request')
def test_http_action(self, mocked_method):
mocked_method.return_value = get_success_fake_response()
mock_ctx = mock.Mock()
action = std.MistralHTTPAction(
url=URL,
method='POST',
body=DATA,
timeout=20,
allow_redirects=True
)
DATA_STR = json.dumps(DATA)
self.assertEqual(DATA_STR, action.body)
self.assertEqual(URL, action.url)
result = action.run(mock_ctx)
self.assertIsInstance(result, dict)
self.assertEqual(DATA, result['content'])
self.assertIn('headers', result)
self.assertEqual(200, result['status'])
mock_ex = mock_ctx.execution
headers = {
'Mistral-Workflow-Name': mock_ex.workflow_name,
'Mistral-Task-Id': mock_ex.task_execution_id,
'Mistral-Callback-URL': mock_ex.callback_url,
'Mistral-Action-Execution-Id': mock_ex.action_execution_id,
'Mistral-Workflow-Execution-Id': mock_ex.workflow_execution_id
}
mocked_method.assert_called_with(
'POST',
URL,
data=DATA_STR,
headers=headers,
cookies=None,
params=None,
timeout=20,
auth=None,
allow_redirects=True,
proxies=None,
verify=None
)
@mock.patch.object(requests, 'request')
def test_http_action_error_result(self, mocked_method):
mocked_method.return_value = get_error_fake_response()
mock_ctx = mock.Mock()
action = std.MistralHTTPAction(
url=URL,
method='POST',
body=DATA,
timeout=20,
allow_redirects=True
)
result = action.run(mock_ctx)
self.assertIsInstance(result, mistral_lib_actions.Result)
self.assertEqual(401, result.error['status'])

View File

@ -38,7 +38,7 @@ wf:
tasks:
task1:
action: my_action
action: {action_name}
input:
success_result: <% $.success_result %>
error_result: <% $.error_result %>
@ -71,14 +71,21 @@ class MyAction(actions_base.Action):
raise NotImplementedError
class MyAsyncAction(MyAction):
def is_sync(self):
return False
class ErrorResultTest(base.EngineTestCase):
def setUp(self):
super(ErrorResultTest, self).setUp()
test_base.register_action_class('my_action', MyAction)
test_base.register_action_class('my_async_action', MyAsyncAction)
def test_error_result1(self):
wf_service.create_workflows(WF)
wf_service.create_workflows(WF.format(action_name="my_action"))
# Start workflow.
wf_ex = self.engine.start_workflow(
@ -111,7 +118,7 @@ class ErrorResultTest(base.EngineTestCase):
self.assertEqual(2, data_flow.get_task_execution_result(task1))
def test_error_result2(self):
wf_service.create_workflows(WF)
wf_service.create_workflows(WF.format(action_name="my_action"))
# Start workflow.
wf_ex = self.engine.start_workflow(
@ -144,7 +151,7 @@ class ErrorResultTest(base.EngineTestCase):
self.assertEqual(3, data_flow.get_task_execution_result(task1))
def test_success_result(self):
wf_service.create_workflows(WF)
wf_service.create_workflows(WF.format(action_name="my_action"))
# Start workflow.
wf_ex = self.engine.start_workflow(
@ -176,3 +183,45 @@ class ErrorResultTest(base.EngineTestCase):
'success',
data_flow.get_task_execution_result(task1)
)
def test_async_error_result(self):
wf_service.create_workflows(WF.format(action_name="my_async_action"))
# Start workflow.
wf_ex = self.engine.start_workflow(
'wf',
wf_input={
'success_result': None,
'error_result': 2
}
)
# If the action errors, we expect the workflow to continue. The
# on-error means the workflow ends in success.
self.await_workflow_success(wf_ex.id)
def test_async_success_result(self):
wf_service.create_workflows(WF.format(action_name="my_async_action"))
# Start workflow.
wf_ex = self.engine.start_workflow(
'wf',
wf_input={
'success_result': 'success',
'error_result': None
}
)
# When the action is successful, the workflow will wait in the RUNNING
# state for it to complete.
self.await_workflow_running(wf_ex.id)
with db_api.transaction():
# Note: We need to reread execution to access related tasks.
wf_ex = db_api.get_workflow_execution(wf_ex.id)
tasks = wf_ex.task_executions
self.assertEqual(1, len(tasks))
task1 = self._assert_single_item(tasks, name='task1')
self.assertEqual(states.RUNNING, task1.state)