diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 965ea703e6..32380c7008 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,7 +6,15 @@ in development * Update ``st2 run`` / ``st2 execution run`` command to display result of workflow actions when they finish. In the workflow case, result of the last task (action) of the workflow is used. - (improvement) + (improvement) #3481 +* Update Python runner so it mimics behavior from StackStorm pre 1.6 and returns action result as + is (serialized as string) in case we are unable to serialize action result because it contains + non-simple types (e.g. class instances) which can't be serialized. + + In v1.6 we introduced a change when in such instances, we simply returned ``None`` as result + and didn't log anything which was confusing. (improvement) #3489 + + Reported by Anthony Shaw. 2.3.0 - June 19, 2017 --------------------- diff --git a/contrib/runners/python_runner/python_runner.py b/contrib/runners/python_runner/python_runner.py index dc5f5f7d1d..221a38135a 100644 --- a/contrib/runners/python_runner/python_runner.py +++ b/contrib/runners/python_runner/python_runner.py @@ -14,6 +14,7 @@ # limitations under the License. import os +import re import sys import json import uuid @@ -178,12 +179,23 @@ def _get_output_values(self, exit_code, stdout, stderr, timed_out): # Parse the serialized action result object try: action_result = json.loads(action_result) - except: - pass + except Exception as e: + # Failed to de-serialize the result, probably it contains non-simple type or similar + LOG.warning('Failed to de-serialize result "%s": %s' % (str(action_result), str(e))) + + if action_result: + if isinstance(action_result, dict): + result = action_result.get('result', None) + status = action_result.get('status', None) + else: + # Failed to de-serialize action result aka result is a string + match = re.search("'result': (.*?)$", action_result or '') + + if match: + action_result = match.groups()[0] - if action_result and isinstance(action_result, dict): - result = action_result.get('result', None) - status = action_result.get('status', None) + result = action_result + status = None else: result = 'None' status = None diff --git a/contrib/runners/python_runner/tests/unit/test_pythonrunner.py b/contrib/runners/python_runner/tests/unit/test_pythonrunner.py index 704fd99416..1fa1e8d0e2 100644 --- a/contrib/runners/python_runner/tests/unit/test_pythonrunner.py +++ b/contrib/runners/python_runner/tests/unit/test_pythonrunner.py @@ -14,6 +14,7 @@ # limitations under the License. import os +import re import mock @@ -42,6 +43,8 @@ 'packs/dummy_pack_9/actions/list_repos_doesnt_exist.py') ACTION_2_PATH = os.path.join(tests_base.get_fixtures_path(), 'packs/dummy_pack_9/actions/invalid_syntax.py') +NON_SIMPLE_TYPE_ACTION = os.path.join(tests_base.get_resources_path(), 'packs', + 'pythonactions/actions/non_simple_type.py') # Note: runner inherits parent args which doesn't work with tests since test pass additional # unrecognized args @@ -59,6 +62,25 @@ def test_runner_creation(self): self.assertTrue(runner is not None, 'Creation failed. No instance.') self.assertEqual(type(runner), python_runner.PythonRunner, 'Creation failed. No instance.') + def test_action_returns_non_serializable_result(self): + # Actions returns non-simple type which can't be serialized, verify result is simple str() + # representation of the result + runner = python_runner.get_runner() + runner.action = self._get_mock_action_obj() + runner.runner_parameters = {} + runner.entry_point = NON_SIMPLE_TYPE_ACTION + runner.container_service = service.RunnerContainerService() + runner.pre_run() + (status, output, _) = runner.run({}) + + self.assertEqual(status, LIVEACTION_STATUS_SUCCEEDED) + self.assertTrue(output is not None) + + expected_result_re = (r"\[{'a': '1'}, {'h': 3, 'c': 2}, {'e': " + "}\]") + match = re.match(expected_result_re, output['result']) + self.assertTrue(match) + def test_simple_action_with_result_no_status(self): runner = python_runner.get_runner() runner.action = self._get_mock_action_obj() diff --git a/st2tests/st2tests/resources/packs/pythonactions/actions/non_simple_type.py b/st2tests/st2tests/resources/packs/pythonactions/actions/non_simple_type.py new file mode 100644 index 0000000000..3a4328aa37 --- /dev/null +++ b/st2tests/st2tests/resources/packs/pythonactions/actions/non_simple_type.py @@ -0,0 +1,15 @@ +from st2common.runners.base_action import Action + + +class Test(object): + foo = 'bar' + + +class NonSimpleTypeAction(Action): + def run(self): + result = [ + {'a': '1'}, + {'c': 2, 'h': 3}, + {'e': Test()} + ] + return result