From f8624a136b77c68dbf3e75406f2903586cf0762b Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Wed, 16 Sep 2015 15:22:25 -0700 Subject: [PATCH] Fix '_cache_get' multiple keyword argument name overlap The argument 'atom_name' is also used by the fetch function so when it is provided a conflict occurs and this ends badly. To avoid this capture the needed variables used for fetching a cached value into a functools.partial object and use that instead of passing further arguments. Closes-Bug: #1496608 Change-Id: Ic012f7687037bf876d041c4bc62b3f6606a8a845 --- taskflow/formatters.py | 17 ++--- taskflow/tests/unit/test_formatters.py | 102 +++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 10 deletions(-) create mode 100644 taskflow/tests/unit/test_formatters.py diff --git a/taskflow/formatters.py b/taskflow/formatters.py index d36082b2d..33fb70882 100644 --- a/taskflow/formatters.py +++ b/taskflow/formatters.py @@ -79,18 +79,15 @@ class FailureFormatter(object): if state_found: atom_attrs['state'] = state if atom_name not in self._hide_inputs_outputs_of: + # When the cache does not exist for this atom this + # will be called with the rest of these arguments + # used to populate the cache. + fetch_mapped_args = functools.partial( + storage.fetch_mapped_args, atom.rebind, + atom_name=atom_name, optional_args=atom.optional) requires, requires_found = _cached_get(cache, 'requires', atom_name, - # When the cache does not - # exist for this atom this - # will be called with the - # rest of these arguments - # used to populate the - # cache. - storage.fetch_mapped_args, - atom.rebind, - atom_name=atom_name, - optional_args=atom.optional) + fetch_mapped_args) if requires_found: atom_attrs['requires'] = requires provides, provides_found = _cached_get(cache, 'provides', diff --git a/taskflow/tests/unit/test_formatters.py b/taskflow/tests/unit/test_formatters.py new file mode 100644 index 000000000..c4db9513e --- /dev/null +++ b/taskflow/tests/unit/test_formatters.py @@ -0,0 +1,102 @@ +# -*- coding: utf-8 -*- + +# Copyright (C) 2015 Yahoo! Inc. All Rights Reserved. +# +# 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. + +from taskflow import engines +from taskflow import formatters +from taskflow.listeners import logging as logging_listener +from taskflow.patterns import linear_flow +from taskflow import states +from taskflow import test +from taskflow.test import mock +from taskflow.test import utils as test_utils + + +class FormattersTest(test.TestCase): + + @staticmethod + def _broken_atom_matcher(node): + return node.item.name == 'Broken' + + def _make_test_flow(self): + b = test_utils.TaskWithFailure("Broken") + h_1 = test_utils.ProgressingTask("Happy-1") + h_2 = test_utils.ProgressingTask("Happy-2") + flo = linear_flow.Flow("test") + flo.add(h_1, h_2, b) + return flo + + def test_exc_info_format(self): + flo = self._make_test_flow() + e = engines.load(flo) + self.assertRaises(RuntimeError, e.run) + + fails = e.storage.get_execute_failures() + self.assertEqual(1, len(fails)) + self.assertIn('Broken', fails) + fail = fails['Broken'] + + f = formatters.FailureFormatter(e) + (exc_info, details) = f.format(fail, self._broken_atom_matcher) + self.assertEqual(3, len(exc_info)) + self.assertEqual("", details) + + @mock.patch('taskflow.formatters.FailureFormatter._format_node') + def test_exc_info_with_details_format(self, mock_format_node): + mock_format_node.return_value = 'A node' + + flo = self._make_test_flow() + e = engines.load(flo) + self.assertRaises(RuntimeError, e.run) + fails = e.storage.get_execute_failures() + self.assertEqual(1, len(fails)) + self.assertIn('Broken', fails) + fail = fails['Broken'] + + # Doing this allows the details to be shown... + e.storage.set_atom_intention("Broken", states.EXECUTE) + f = formatters.FailureFormatter(e) + (exc_info, details) = f.format(fail, self._broken_atom_matcher) + self.assertEqual(3, len(exc_info)) + self.assertTrue(mock_format_node.called) + + @mock.patch('taskflow.storage.Storage.get_execute_result') + def test_exc_info_with_details_format_hidden(self, mock_get_execute): + flo = self._make_test_flow() + e = engines.load(flo) + self.assertRaises(RuntimeError, e.run) + fails = e.storage.get_execute_failures() + self.assertEqual(1, len(fails)) + self.assertIn('Broken', fails) + fail = fails['Broken'] + + # Doing this allows the details to be shown... + e.storage.set_atom_intention("Broken", states.EXECUTE) + hide_inputs_outputs_of = ['Broken', "Happy-1", "Happy-2"] + f = formatters.FailureFormatter( + e, hide_inputs_outputs_of=hide_inputs_outputs_of) + (exc_info, details) = f.format(fail, self._broken_atom_matcher) + self.assertEqual(3, len(exc_info)) + self.assertFalse(mock_get_execute.called) + + @mock.patch('taskflow.formatters.FailureFormatter._format_node') + def test_formatted_via_listener(self, mock_format_node): + mock_format_node.return_value = 'A node' + + flo = self._make_test_flow() + e = engines.load(flo) + with logging_listener.DynamicLoggingListener(e): + self.assertRaises(RuntimeError, e.run) + self.assertTrue(mock_format_node.called)