openstack
/
rally
Code Issues Proposed changes

Fix usage of InvalidTaskException in the task engine 

The current implementation of task validation wraps all exceptions with
InvalidTaskException class. There are bunch of expected errors which
produce user-friendly errors and there is no reason to wrap them. Also,
since they are raised in expected places, printing traceback of such
exceptions even in debug mode is redundant.
As for unexpected errors, they can happen only while developing new
features or modifying existing ones. Wrapping them by
InvalidTaskException hiddes traceback by default and it is hard to
identify the real root cause. Let's remove usage of InvalidTaskException
at validation step to provide more clear messages.

Change-Id: I3795589c3dd0c6dae1198f303a6f14f94c82db2c
This commit is contained in:
Andrey Kurilin 2018-09-09 14:36:39 +03:00
parent 49c4527b03
commit 6ebba91c9c
3 changed files with 26 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
rally/api.py
View File

@ -360,7 +360,7 @@ class _Task(APIGroup):
raise
except Exception as e:
if logging.is_debug():
LOG.exception("Invalid Task")
LOG.exception("Unexpected error had happened")
raise exceptions.InvalidTaskException(str(e))
engine.TaskEngine(config, task, deployment.env_obj).validate()
@ -407,7 +407,7 @@ class _Task(APIGroup):
raise
except Exception as e:
if logging.is_debug():
LOG.exception("Invalid Task")
LOG.exception("Unexpected error had happened")
raise exceptions.InvalidTaskException(str(e))
if task is None:

16
rally/task/engine.py
View File

@ -396,10 +396,18 @@ class TaskEngine(object):
exception_info = json.dumps(traceback.format_exc(), indent=2,
separators=(",", ": "))
self.task.set_failed(type(e).__name__, str(e), exception_info)
if (logging.is_debug() and
not isinstance(e, exceptions.InvalidTaskConfig)):
LOG.exception("Invalid Task")
raise exceptions.InvalidTaskException(str(e))
expected_errors = (
# this error is a wrapper for all error messages from
# validators.
exceptions.InvalidTaskConfig,
# rally.task.task_cfg raises it
# _validate_config_semantic raises this error in case of
# failed platform check{s}
exceptions.ValidationError)
if logging.is_debug() and not isinstance(e, expected_errors):
LOG.exception("Unexpected error had happened while validating "
"task.")
raise
def _prepare_context(self, ctx, scenario_name, owner_id):
context_config = {}

18
tests/unit/task/test_engine.py
View File

@ -77,11 +77,14 @@ class TaskEngineTestCase(test.TestCase):
def test_validate__wrong_syntax(self):
task = mock.MagicMock()
eng = engine.TaskEngine(mock.MagicMock(), task, mock.Mock())
eng._validate_config_syntax = mock.MagicMock(
side_effect=exceptions.InvalidTaskConfig)
e = exceptions.InvalidTaskConfig(name="foo", pos=0, config="",
reason="foo")
eng._validate_config_syntax = mock.MagicMock(side_effect=e)
eng._validate_config_platforms = mock.Mock()
self.assertRaises(exceptions.InvalidTaskException, eng.validate)
actual_e = self.assertRaises(exceptions.InvalidTaskException,
eng.validate)
self.assertEqual(e, actual_e)
self.assertTrue(task.set_failed.called)
# the next validation step should not be processed
@ -90,12 +93,15 @@ class TaskEngineTestCase(test.TestCase):
def test_validate__wrong_semantic(self):
task = mock.MagicMock()
eng = engine.TaskEngine(mock.MagicMock(), task, mock.Mock())
e = exceptions.InvalidTaskConfig(name="foo", pos=0, config="",
reason="foo")
eng._validate_config_syntax = mock.MagicMock()
eng._validate_config_platforms = mock.MagicMock()
eng._validate_config_semantic = mock.MagicMock(
side_effect=exceptions.InvalidTaskConfig)
eng._validate_config_semantic = mock.MagicMock(side_effect=e)
self.assertRaises(exceptions.InvalidTaskException, eng.validate)
actual_e = self.assertRaises(exceptions.InvalidTaskException,
eng.validate)
self.assertEqual(e, actual_e)
self.assertTrue(task.set_failed.called)
# all steps of validation are called, which means that the last one is
# failed