From b0e3942c7ccc4253c03620f4285b03cfff26a692 Mon Sep 17 00:00:00 2001 From: James Johnson <jjohnson@enthought.com> Date: Thu, 12 Jul 2018 15:53:26 +0100 Subject: [PATCH] Ported code from workflow_tree.py in force_wfmanager --- force_bdss/core/tests/test_verifier.py | 31 ++--- force_bdss/core/verifier.py | 139 +++++++++++++++++------ force_bdss/core_mco_driver.py | 2 +- force_bdss/tests/test_core_mco_driver.py | 2 +- 4 files changed, 126 insertions(+), 48 deletions(-) diff --git a/force_bdss/core/tests/test_verifier.py b/force_bdss/core/tests/test_verifier.py index 7d6e234..74123b7 100644 --- a/force_bdss/core/tests/test_verifier.py +++ b/force_bdss/core/tests/test_verifier.py @@ -20,10 +20,10 @@ class TestVerifier(unittest.TestCase): errors = verify_workflow(wf) self.assertEqual(len(errors), 2) self.assertEqual(errors[0].subject, wf) - self.assertIn("no MCO", errors[0].error) + self.assertIn("no MCO", errors[0].local_error) self.assertEqual(errors[1].subject, wf) - self.assertIn("no execution layers", errors[1].error) + self.assertIn("no execution layers", errors[1].local_error) def test_no_mco_parameters(self): wf = self.workflow @@ -32,7 +32,7 @@ class TestVerifier(unittest.TestCase): errors = verify_workflow(wf) self.assertEqual(len(errors), 2) self.assertEqual(errors[0].subject, wf.mco) - self.assertIn("no defined parameters", errors[0].error) + self.assertIn("no defined parameters", errors[0].local_error) def test_empty_parameter_options(self): wf = self.workflow @@ -44,9 +44,9 @@ class TestVerifier(unittest.TestCase): errors = verify_workflow(wf) self.assertEqual(len(errors), 3) self.assertEqual(errors[0].subject, wf.mco.parameters[0]) - self.assertIn("Empty Name", errors[0].error) + self.assertIn("MCO parameter is not named", errors[0].local_error) self.assertEqual(errors[1].subject, wf.mco.parameters[0]) - self.assertIn("Empty Type", errors[1].error) + self.assertIn("MCO parameter has no type set", errors[1].local_error) def test_empty_kpi_options(self): wf = self.workflow @@ -59,8 +59,8 @@ class TestVerifier(unittest.TestCase): self.assertEqual(len(errors), 4) self.assertEqual(errors[1].subject, wf.mco.kpis[0]) - self.assertIn("Empty Name", errors[1].error) - self.assertIn("Empty Objective", errors[2].error) + self.assertIn("KPI is not named", errors[1].local_error) + self.assertIn("KPI has no objective set", errors[2].local_error) def test_empty_execution_layer(self): wf = self.workflow @@ -74,9 +74,10 @@ class TestVerifier(unittest.TestCase): layer = ExecutionLayer() wf.execution_layers.append(layer) errors = verify_workflow(wf) - self.assertEqual(len(errors), 1) + + self.assertEqual(len(errors), 2) self.assertEqual(errors[0].subject, wf.execution_layers[0]) - self.assertIn("Layer 0 has no data sources", errors[0].error) + self.assertIn("Layer 0 has no data sources", errors[1].local_error) def test_data_sources(self): wf = self.workflow @@ -95,7 +96,8 @@ class TestVerifier(unittest.TestCase): errors = verify_workflow(wf) self.assertEqual(errors[0].subject, ds_model) - self.assertIn("Missing input slot name assignment", errors[0].error) + self.assertIn("Missing input slot name assignment in layer 0", + errors[0].local_error) ds_model.input_slot_info.append( InputSlotInfo(name="name") @@ -103,7 +105,8 @@ class TestVerifier(unittest.TestCase): errors = verify_workflow(wf) self.assertEqual(errors[0].subject, ds_model) - self.assertIn("Missing output slot name assignment", errors[0].error) + self.assertIn("Missing output slot name assignment in layer 0", + errors[0].local_error) ds_model.output_slot_info.append( OutputSlotInfo(name="name") @@ -115,9 +118,11 @@ class TestVerifier(unittest.TestCase): ds_model.input_slot_info[0].name = '' errors = verify_workflow(wf) self.assertEqual(len(errors), 1) - self.assertIn("Undefined name for input parameter", errors[0].error) + self.assertIn("Undefined name for input parameter", + errors[0].local_error) ds_model.output_slot_info[0].name = '' errors = verify_workflow(wf) self.assertEqual(len(errors), 2) - self.assertIn("Undefined name for output parameter", errors[1].error) + self.assertIn("Undefined name for output parameter", + errors[1].local_error) diff --git a/force_bdss/core/verifier.py b/force_bdss/core/verifier.py index 2526c4e..84b68b4 100644 --- a/force_bdss/core/verifier.py +++ b/force_bdss/core/verifier.py @@ -1,4 +1,7 @@ import logging + +from itertools import groupby + from traits.api import HasStrictTraits, Str, Any logger = logging.getLogger(__name__) @@ -6,7 +9,18 @@ logger = logging.getLogger(__name__) class VerifierError(HasStrictTraits): subject = Any() - error = Str() + #: An error message relevant to the local modelview + local_error = Str() + #: An error message relevent to the overall workflow + global_error = Str() + + def __init__(self, subject, global_error='', local_error=''): + self.subject = subject + if local_error == '': + self.local_error = global_error + else: + self.local_error = local_error + self.global_error = global_error def verify_workflow(workflow): @@ -26,39 +40,42 @@ def _check_mco(workflow): errors.append( VerifierError( subject=workflow, - error="Workflow has no MCO" + local_error="Workflow has no MCO", ) ) return errors mco = workflow.mco if len(mco.parameters) == 0: - errors.append(VerifierError(subject=mco, - error="MCO has no defined parameters")) + errors.append(VerifierError( + subject=mco, + global_error="The MCO has no defined parameters")) for idx, param in enumerate(mco.parameters): factory_name = param.factory.name if param.name == '': - errors.append(VerifierError(subject=param, - error="Empty Name - Parameter {} " - "({})".format(idx, - factory_name))) + errors.append(VerifierError( + subject=param, + local_error="MCO parameter is not named", + global_error="Error in MCO parameter " + "(Type: {})".format(factory_name))) if len(param.type.strip()) == 0: - errors.append(VerifierError(subject=param, - error="Empty Type - Parameter {} " - "({})".format(idx, - factory_name))) + errors.append(VerifierError( + subject=param, + local_error="MCO parameter has no type set", + global_error="Error in MCO parameter " + "(Type: {})".format(factory_name))) for idx, kpi in enumerate(mco.kpis): if kpi.name == '': errors.append(VerifierError(subject=kpi, - error="Empty Name - KPI {}".format( - idx))) + local_error="KPI is not named", + global_error="A KPI has an error")) if kpi.objective == '': errors.append(VerifierError(subject=kpi, - error="Empty Objective - " - "KPI {}".format(idx))) + local_error="KPI has no objective set", + global_error="A KPI has an error")) return errors @@ -71,21 +88,34 @@ def _check_execution_layers(workflow): errors.append( VerifierError( subject=workflow, - error="Workflow has no execution layers" + global_error="Workflow has no execution layers" ) ) return errors + layer_index_errors = [] for idx, layer in enumerate(layers): if len(layer.data_sources) == 0: - errors.append(VerifierError(subject=layer, - error="Layer {} has no " - "data sources".format(idx))) + layer_index_errors.append(idx) + errors.append(VerifierError( + subject=layer, local_error="Layer has no data sources")) for ds in layer.data_sources: errors.extend(_check_data_source(ds, idx)) + if layer_index_errors != []: + multi_str = multi_error_format(layer_index_errors) + if len(layer_index_errors) == 1: + errors.append(VerifierError( + subject=workflow, + local_error="Layer {} has no data sources".format(multi_str))) + else: + errors.append(VerifierError( + subject=workflow, + local_error="Layers {} have no data " + "sources".format(multi_str))) + return errors @@ -116,31 +146,74 @@ def _check_data_source(data_source_model, layer_number): if len(input_slots) != len(data_source_model.input_slot_info): errors.append(VerifierError( subject=data_source_model, - error="Missing input slot name assignment " - "in layer {}".format(layer_number))) + global_error="Missing input slot name assignment " + "in layer {}".format(layer_number))) + row_index_errors = [] for idx, info in enumerate(data_source_model.input_slot_info): if info.name == '': + row_index_errors.append(idx) + if row_index_errors != []: + err_no_string = multi_error_format(row_index_errors) + if len(row_index_errors) == 1: + errors.append(VerifierError( + subject=data_source_model, + local_error="Undefined name for input parameter " + "{}".format(err_no_string), + global_error="An input parameter is undefined in {} " + "(Layer {})".format(factory.name, layer_number))) + else: errors.append(VerifierError( subject=data_source_model, - error="Undefined name for input " - "parameter {} from {} in layer {}".format(idx, - factory.name, - layer_number))) + local_error="Undefined name for input parameters " + "{}".format(err_no_string), + global_error="An input parameter is undefined in {} " + "(Layer {})".format(factory.name, layer_number))) if len(output_slots) != len(data_source_model.output_slot_info): errors.append(VerifierError( subject=data_source_model, - error="Missing output slot name assignment " - "in layer {}".format(layer_number))) + global_error="Missing output slot name assignment " + "in layer {}".format(layer_number))) + row_index_errors = [] for idx, info in enumerate(data_source_model.output_slot_info): if info.name == '': - errors.append(VerifierError( + row_index_errors.append(idx) + err_no_string = multi_error_format(row_index_errors) + if len(row_index_errors) == 1: + errors.append(VerifierError( subject=data_source_model, - error="Undefined name for output " - "parameter {} from {} in layer {}".format(idx, - factory.name, - layer_number))) + local_error="Undefined name for output " + "parameters {}".format(err_no_string), + global_error="An output parameter is undefined in {}" + " (Layer {})".format(factory.name, layer_number))) return errors + + +def multi_error_format(index_list): + """Takes a list of integers and returns a string where they are grouped + consecutively wherever possible. + For example an input of [0,1,2,4,5,7] returns the string '0-2, 4-5, 7' """ + index_list.sort() + # Single, consecutive or non-consecutive + if len(index_list) == 1: + return str(index_list[0]) + else: + repl = [] + + for i, index_group in groupby(enumerate(index_list), lambda val: + val[0]-val[1]): + group_index_list = [] + for enum_idx, error_idx in index_group: + group_index_list.append(error_idx) + if len(group_index_list) == 1: + repl.append(str(group_index_list[0])) + else: + repl.append('{}-{}'.format(group_index_list[0], + group_index_list[-1])) + # Conversion from list of strings to comma separated string + return_string = ', '.join(repl) + + return return_string diff --git a/force_bdss/core_mco_driver.py b/force_bdss/core_mco_driver.py index 46b9cda..1abdceb 100644 --- a/force_bdss/core_mco_driver.py +++ b/force_bdss/core_mco_driver.py @@ -38,7 +38,7 @@ class CoreMCODriver(BaseCoreDriver): log.error("Unable to execute workflow due to verification " "errors :") for err in errors: - log.error(err.error) + log.error(err.local_error) sys.exit(1) try: diff --git a/force_bdss/tests/test_core_mco_driver.py b/force_bdss/tests/test_core_mco_driver.py index 421d5de..fc325cd 100644 --- a/force_bdss/tests/test_core_mco_driver.py +++ b/force_bdss/tests/test_core_mco_driver.py @@ -231,7 +231,7 @@ class TestCoreMCODriver(unittest.TestCase): 'ERROR', 'Unable to execute workflow due to verification errors :'), ('force_bdss.core_mco_driver', 'ERROR', - 'MCO has no defined parameters'), + 'The MCO has no defined parameters'), ('force_bdss.core_mco_driver', 'ERROR', 'Missing input slot name assignment in layer 0'), ('force_bdss.core_mco_driver', 'ERROR', -- GitLab