From dd0f984347792c162bb617e2e4fb0395e3156132 Mon Sep 17 00:00:00 2001 From: Dan Funk Date: Mon, 20 Jul 2020 12:26:34 -0400 Subject: [PATCH] Drop backwards compatibility of scripts. While this will cause some initial pain, it's less confusing and error prone, and we are still in the development phase of the project. Were this going straight to production we would likely want to keep this backwards compatibility. Don't parse on spaces if this is python code, so we avoid any errors in processing - spaces should be valid. --- crc/services/workflow_processor.py | 34 ++++++++----------- .../bpmn/research_rampup/research_rampup.bpmn | 2 +- .../test_workflow_spec_validation_api.py | 3 -- 3 files changed, 16 insertions(+), 23 deletions(-) diff --git a/crc/services/workflow_processor.py b/crc/services/workflow_processor.py index 63113aa7..165d3313 100644 --- a/crc/services/workflow_processor.py +++ b/crc/services/workflow_processor.py @@ -33,27 +33,27 @@ class CustomBpmnScriptEngine(BpmnScriptEngine): def execute(self, task: SpiffTask, script, data): """ - Assume that the script read in from the BPMN file is a fully qualified python class. Instantiate - that class, pass in any data available to the current task so that it might act on it. - Assume that the class implements the "do_task" method. - - This allows us to reference custom code from the BPMN diagram. + Functions in two modes. + 1. If the command is proceeded by #! then this is assumed to be a python script, and will + attempt to load that python module and execute the do_task method on that script. Scripts + must be located in the scripts package and they must extend the script.py class. + 2. If not proceeded by the #! this will attempt to execute the script directly and assumes it is + valid Python. """ # Shlex splits the whole string while respecting double quoted strings within - commands = shlex.split(script) - failedOnce = False - prevError = '' - if commands[0] != '#!': + if not script.startswith('#!'): try: - super().execute(task,script,data) + super().execute(task, script, data) except SyntaxError as e: - failedOnce = True - prevError = script - app.logger.warning('We experienced a syntax error, but we are going to try the old method on ' - '"%s"'%script) + raise ApiError.from_task('syntax_error', + f'If you are running a pre-defined script, please' + f' proceed the script with "#!", otherwise this is assumed to be' + f' pure python: {script}, {e.msg}', task=task) else: - commands = commands[1:] + self.run_predefined_script(task, script[2:], data) # strip off the first two characters. + def run_predefined_script(self, task: SpiffTask, script, data): + commands = shlex.split(script) path_and_command = commands[0].rsplit(".", 1) if len(path_and_command) == 1: module_name = "crc.scripts." + self.camel_to_snake(path_and_command[0]) @@ -83,10 +83,6 @@ class CustomBpmnScriptEngine(BpmnScriptEngine): else: klass().do_task(task, study_id, workflow_id, *commands[1:]) except ModuleNotFoundError: - if failedOnce: - raise ApiError.from_task("invalid_script", - "Script had a syntax error: '%s'" % (prevError), - task=task) raise ApiError.from_task("invalid_script", "Unable to locate Script: '%s:%s'" % (module_name, class_name), task=task) diff --git a/crc/static/bpmn/research_rampup/research_rampup.bpmn b/crc/static/bpmn/research_rampup/research_rampup.bpmn index 5a4bb1bc..4a04eb6d 100644 --- a/crc/static/bpmn/research_rampup/research_rampup.bpmn +++ b/crc/static/bpmn/research_rampup/research_rampup.bpmn @@ -755,7 +755,7 @@ Notify the Area Monitor for This step is internal to the system and do not require and user interaction Flow_0j4rs82 Flow_07ge8uf - RequestApproval ApprvlApprvr1 ApprvlApprvr2 + #!RequestApproval ApprvlApprvr1 ApprvlApprvr2 #### Script Task diff --git a/tests/workflow/test_workflow_spec_validation_api.py b/tests/workflow/test_workflow_spec_validation_api.py index 597c4aa1..00406f5b 100644 --- a/tests/workflow/test_workflow_spec_validation_api.py +++ b/tests/workflow/test_workflow_spec_validation_api.py @@ -66,7 +66,6 @@ class TestWorkflowSpecValidation(BaseTest): errors.extend(ApiErrorSchema(many=True).load(json_data)) self.assertEqual(0, len(errors), json.dumps(errors)) - def test_invalid_expression(self): self.load_example_data() errors = self.validate_workflow("invalid_expression") @@ -103,12 +102,10 @@ class TestWorkflowSpecValidation(BaseTest): errors = self.validate_workflow("invalid_script2") self.assertEqual(2, len(errors)) self.assertEqual("error_loading_workflow", errors[0]['code']) - self.assertTrue("syntax error" in errors[0]['message']) self.assertEqual("Invalid_Script_Task", errors[0]['task_id']) self.assertEqual("An Invalid Script Reference", errors[0]['task_name']) self.assertEqual("invalid_script2.bpmn", errors[0]['file_name']) - def test_repeating_sections_correctly_populated(self): self.load_example_data() spec_model = self.load_test_spec('repeat_form')