Skip to content

Commit

Permalink
Fix mistral callback failure when result contains unicode
Browse files Browse the repository at this point in the history 
When result of action execution contains unicode, Mistral callback fails and leads to orphaned workflow execution.
  • Loading branch information
@m4dcoder
m4dcoder committed Sep 7, 2017
1 parent e0d9afc commit bbbaeb7
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Fixed
performed the API operation.(bug fix) #3693 #3696

Reported by theuiz.
* Fix mistral callback failure when result contains unicode. (bug fix)

Changed
~~~~~~~
Expand Down
24 changes: 24 additions & 0 deletions contrib/runners/mistral_v2/callback/mistral_v2.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import json
import re
import retrying
import six

from oslo_config import cfg
from mistralclient.api import client as mistral
Expand Down Expand Up @@ -88,6 +89,27 @@ def _update_action_execution(cls, url, data):

client.action_executions.update(action_execution_id, **data)

@classmethod
def _encode(cls, value):
if isinstance(value, dict):
return {k: cls._encode(v) for k, v in six.iteritems(value)}
elif isinstance(value, list):
return [cls._encode(item) for item in value]
elif isinstance(value, six.string_types):
try:
value = value.decode('utf-8')
except Exception:
LOG.exception('Unable to decode value to utf-8.')

try:
value = value.encode('unicode_escape')
except Exception:
LOG.exception('Unable to unicode escape value.')

return value
else:
return value

@classmethod
def callback(cls, url, context, status, result):
if status not in MISTRAL_ACCEPTED_STATES:
Expand All @@ -100,7 +122,9 @@ def callback(cls, url, context, status, result):
if type(value) in [dict, list]:
result = value

result = cls._encode(result)
output = json.dumps(result) if type(result) in [dict, list] else str(result)
output = output.replace('\\\\\\\\u', '\\\\u')
data = {'state': STATUS_MAP[status], 'output': output}

cls._update_action_execution(url, data)
Expand Down
2 changes: 1 addition & 1 deletion contrib/runners/mistral_v2/tests/unit/test_mistral_v2_auth.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ def test_launch_workflow_with_mistral_auth(self):
'cacert': None
}

keystone.KeystoneAuthHandler.authenticate.assert_called_with(auth_req)
keystone.KeystoneAuthHandler.authenticate.assert_called_with(auth_req, session=None)

executions.ExecutionManager.create.assert_called_with(
WF1_NAME, workflow_input=workflow_input, env=env)
119 changes: 119 additions & 0 deletions contrib/runners/mistral_v2/tests/unit/test_mistral_v2_callback.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,22 +110,47 @@ def test_callback_handler_with_result_as_text(self):
action_constants.LIVEACTION_STATUS_SUCCEEDED,
'<html></html>')

action_executions.ActionExecutionManager.update.assert_called_with(
'12345',
state='SUCCESS',
output='<html></html>'
)

@mock.patch.object(
action_executions.ActionExecutionManager, 'update',
mock.MagicMock(return_value=None))
def test_callback_handler_with_result_as_dict(self):
self.callback_class.callback('http://127.0.0.1:8989/v2/action_executions/12345', {},
action_constants.LIVEACTION_STATUS_SUCCEEDED, {'a': 1})

action_executions.ActionExecutionManager.update.assert_called_with(
'12345',
state='SUCCESS',
output='{"a": 1}'
)

@mock.patch.object(
action_executions.ActionExecutionManager, 'update',
mock.MagicMock(return_value=None))
def test_callback_handler_with_result_as_json_str(self):
self.callback_class.callback('http://127.0.0.1:8989/v2/action_executions/12345', {},
action_constants.LIVEACTION_STATUS_SUCCEEDED, '{"a": 1}')

action_executions.ActionExecutionManager.update.assert_called_with(
'12345',
state='SUCCESS',
output='{"a": 1}'
)

self.callback_class.callback('http://127.0.0.1:8989/v2/action_executions/12345', {},
action_constants.LIVEACTION_STATUS_SUCCEEDED, "{'a': 1}")

action_executions.ActionExecutionManager.update.assert_called_with(
'12345',
state='SUCCESS',
output='{"a": 1}'
)

@mock.patch.object(
action_executions.ActionExecutionManager, 'update',
mock.MagicMock(return_value=None))
Expand All @@ -134,6 +159,12 @@ def test_callback_handler_with_result_as_list(self):
action_constants.LIVEACTION_STATUS_SUCCEEDED,
["a", "b", "c"])

action_executions.ActionExecutionManager.update.assert_called_with(
'12345',
state='SUCCESS',
output='["a", "b", "c"]'
)

@mock.patch.object(
action_executions.ActionExecutionManager, 'update',
mock.MagicMock(return_value=None))
Expand All @@ -142,6 +173,94 @@ def test_callback_handler_with_result_as_list_str(self):
action_constants.LIVEACTION_STATUS_SUCCEEDED,
'["a", "b", "c"]')

action_executions.ActionExecutionManager.update.assert_called_with(
'12345',
state='SUCCESS',
output='["a", "b", "c"]'
)

@mock.patch.object(
action_executions.ActionExecutionManager, 'update',
mock.MagicMock(return_value=None))
def test_callback_handler_with_result_unicode_str(self):
self.callback_class.callback('http://127.0.0.1:8989/v2/action_executions/12345', {},
action_constants.LIVEACTION_STATUS_SUCCEEDED, '\u4ec0\u9ebc')

action_executions.ActionExecutionManager.update.assert_called_with(
'12345',
state='SUCCESS',
output='\\\\u4ec0\\\\u9ebc'
)

@mock.patch.object(
action_executions.ActionExecutionManager, 'update',
mock.MagicMock(return_value=None))
def test_callback_handler_with_result_unicode_type(self):
self.callback_class.callback('http://127.0.0.1:8989/v2/action_executions/12345', {},
action_constants.LIVEACTION_STATUS_SUCCEEDED, u'\u4ec0\u9ebc')

action_executions.ActionExecutionManager.update.assert_called_with(
'12345',
state='SUCCESS',
output='\\u4ec0\\u9ebc'
)

@mock.patch.object(
action_executions.ActionExecutionManager, 'update',
mock.MagicMock(return_value=None))
def test_callback_handler_with_result_as_list_with_unicode_str(self):
self.callback_class.callback('http://127.0.0.1:8989/v2/action_executions/12345', {},
action_constants.LIVEACTION_STATUS_SUCCEEDED,
['\u4ec0\u9ebc'])

action_executions.ActionExecutionManager.update.assert_called_with(
'12345',
state='SUCCESS',
output='["\\\\u4ec0\\\\u9ebc"]'
)

@mock.patch.object(
action_executions.ActionExecutionManager, 'update',
mock.MagicMock(return_value=None))
def test_callback_handler_with_result_as_list_with_unicode_type(self):
self.callback_class.callback('http://127.0.0.1:8989/v2/action_executions/12345', {},
action_constants.LIVEACTION_STATUS_SUCCEEDED,
[u'\u4ec0\u9ebc'])

action_executions.ActionExecutionManager.update.assert_called_with(
'12345',
state='SUCCESS',
output='["\\\\u4ec0\\\\u9ebc"]'
)

@mock.patch.object(
action_executions.ActionExecutionManager, 'update',
mock.MagicMock(return_value=None))
def test_callback_handler_with_result_as_dict_with_unicode_str(self):
self.callback_class.callback('http://127.0.0.1:8989/v2/action_executions/12345', {},
action_constants.LIVEACTION_STATUS_SUCCEEDED,
{'a': '\u4ec0\u9ebc'})

action_executions.ActionExecutionManager.update.assert_called_with(
'12345',
state='SUCCESS',
output='{"a": "\\\\u4ec0\\\\u9ebc"}'
)

@mock.patch.object(
action_executions.ActionExecutionManager, 'update',
mock.MagicMock(return_value=None))
def test_callback_handler_with_result_as_dict_with_unicode_type(self):
self.callback_class.callback('http://127.0.0.1:8989/v2/action_executions/12345', {},
action_constants.LIVEACTION_STATUS_SUCCEEDED,
{'a': u'\u4ec0\u9ebc'})

action_executions.ActionExecutionManager.update.assert_called_with(
'12345',
state='SUCCESS',
output='{"a": "\\\\u4ec0\\\\u9ebc"}'
)

@mock.patch.object(
action_executions.ActionExecutionManager, 'update',
mock.MagicMock(return_value=None))
Expand Down

0 comments on commit bbbaeb7

Please sign in to comment.