From 13e1a6576933e3a15ded2579a7ef867680d9edcb Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Fri, 25 Mar 2022 16:28:43 +0000 Subject: [PATCH] xtriggers: reinstate Cylc 7 template variables * Closes https://github.com/cylc/cylc-flow/issues/4757 * Reinstate Cylc 7 xtrigger template variables with deprecation warnings to allow 7/8 compativle xtrigger definitions. * Deprecate `workflow_name` in favor of `workflow` for consistency with event template variables (and because it's `workflow_id` not `workflow_name`). * Raise a special xtrigger config exception that inheritc from CylcError rather than primitive exceptions so make errors format more nicely on the CLI. --- CHANGES.md | 8 ++ cylc/flow/cfgspec/workflow.py | 2 +- cylc/flow/config.py | 1 + cylc/flow/exceptions.py | 22 ++++ cylc/flow/xtrigger_mgr.py | 210 ++++++++++++++++++++++++++------ tests/unit/test_config.py | 11 +- tests/unit/test_xtrigger_mgr.py | 20 ++- 7 files changed, 227 insertions(+), 47 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index e787506da9c..9afa9fc9d51 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -28,6 +28,14 @@ creating a new release entry be sure to copy & paste the span tag with the `actions:bind` attribute, which is used by a regex to find the text to be updated. Only the first match gets replaced, so it's fine to leave the old ones in. --> +------------------------------------------------------------------------------- +## __cylc-8.0rc3 (Pending)__ + +### Fixes: + +[#4777](https://github.com/cylc/cylc-flow/pull/4777) - +Reinstate the Cylc 7 template variables for xtriggers with deprecation warnings. + ------------------------------------------------------------------------------- ## __cylc-8.0rc2 (Released 2022-03-23)__ diff --git a/cylc/flow/cfgspec/workflow.py b/cylc/flow/cfgspec/workflow.py index 6732adad6bb..a4a9231f27c 100644 --- a/cylc/flow/cfgspec/workflow.py +++ b/cylc/flow/cfgspec/workflow.py @@ -797,7 +797,7 @@ def get_script_common_text(this: str, example: Optional[str] = None): Example:: - ``my_trigger(arg1, arg2, kwarg1, kwarg2):PT10S`` + ``my_trigger(arg1, arg2, kwarg1, kwarg2):PT10S`` ''') with Conf('graph', desc=f''' diff --git a/cylc/flow/config.py b/cylc/flow/config.py index 4a7e3d5d573..fadb1edb0a6 100644 --- a/cylc/flow/config.py +++ b/cylc/flow/config.py @@ -1672,6 +1672,7 @@ def generate_triggers(self, lexpression, left_nodes, right, seq, ) for label in xtrig_labels: + try: xtrig = self.cfg['scheduling']['xtriggers'][label] except KeyError: diff --git a/cylc/flow/exceptions.py b/cylc/flow/exceptions.py index 8b8dfead069..8946cd61ed6 100644 --- a/cylc/flow/exceptions.py +++ b/cylc/flow/exceptions.py @@ -223,6 +223,28 @@ class TaskDefError(WorkflowConfigError): """Exception raise for errors in TaskDef initialization.""" +class XtriggerConfigError(WorkflowConfigError): + """An error in an xtrigger. + + For example: + + * If the function module was not found. + * If the function was not found in the xtrigger module. + * If the function is not callable. + * If any string template in the function context + arguments are not present in the expected template values. + + """ + + def __init__(self, label: str, trigger: str, message: str): + self.label: str = label + self.trigger: str = trigger + self.message: str = message + + def __str__(self): + return f'[{self.label}] {self.message}' + + class ClientError(CylcError): def __init__(self, message: str, traceback: Optional[str] = None): diff --git a/cylc/flow/xtrigger_mgr.py b/cylc/flow/xtrigger_mgr.py index 2f14df11644..ae42273acde 100644 --- a/cylc/flow/xtrigger_mgr.py +++ b/cylc/flow/xtrigger_mgr.py @@ -15,6 +15,7 @@ # along with this program. If not, see . from contextlib import suppress +from enum import Enum import json import re from copy import deepcopy @@ -22,6 +23,7 @@ from typing import Any, Dict, List, Optional, Tuple, Callable from cylc.flow import LOG +from cylc.flow.exceptions import XtriggerConfigError import cylc.flow.flags from cylc.flow.hostuserutil import get_user from cylc.flow.xtriggers.wall_clock import wall_clock @@ -34,20 +36,111 @@ from cylc.flow.subprocpool import get_func -# Templates for string replacement in function arg values. -TMPL_USER_NAME = 'user_name' -TMPL_WORKFLOW_NAME = 'workflow_name' -TMPL_TASK_CYCLE_POINT = 'point' -TMPL_TASK_IDENT = 'id' -TMPL_TASK_NAME = 'name' -TMPL_WORKFLOW_RUN_DIR = 'workflow_run_dir' -TMPL_WORKFLOW_SHARE_DIR = 'workflow_share_dir' -TMPL_DEBUG_MODE = 'debug' -ARG_VAL_TEMPLATES: List[str] = [ - TMPL_TASK_CYCLE_POINT, TMPL_TASK_IDENT, TMPL_TASK_NAME, - TMPL_WORKFLOW_RUN_DIR, TMPL_WORKFLOW_SHARE_DIR, TMPL_USER_NAME, - TMPL_WORKFLOW_NAME, TMPL_DEBUG_MODE -] +class TemplateVariables(Enum): + """Templates variables for string replacement in xtrigger functions. + + The following string templates are available for use, if the trigger + function needs any of this information, in function arguments in the + workflow configuration. + + .. code-block:: cylc + + [scheduling] + initial cycle point = now + [[xtriggers]] + my_xtrigger = my_xtrigger_fcn('%(workflow)', '%(point)') + + For an explanation of the substitution syntax, see + `String Formatting Operations in the Python documentation + `_. + + """ + + CyclePoint = 'point' + """The cycle point of the dependent task.""" + + DebugMode = 'debug' + """True if Cylc is being run in debug mode (--debug, -vv).""" + + RunDir = 'workflow_run_dir' + """The path to the workflow run directory.""" + + ShareDir = 'workflow_share_dir' + """The path to the workflow share directory.""" + + TaskID = 'id' + """The ID of the dependent task.""" + + TaskName = 'name' + """The name of the dependent task.""" + + UserName = 'user_name' + """The user account under which the workflow is being run.""" + + Workflow = 'workflow' + """The workflow ID.""" + + # BACK COMPAT: workflow_name deprecated + # url: + # TODO + # from: + # Cylc 8 + # remove at: + # Cylc 9 + WorkflowName = 'workflow_name' + """The workflow ID. + + .. deprecated:: 8.0.0 + + Deprecated, use ``workflow``. + """ + + # BACK COMPAT: suite_name deprecated + # url: + # TODO + # from: + # Cylc 8 + # remove at: + # Cylc 9 + SuiteName = 'suite_name' + """The workflow ID. + + .. deprecated:: 8.0.0 + + Deprecated, use ``workflow``. + """ + + # BACK COMPAT: suite_run_dir deprecated + # url: + # TODO + # from: + # Cylc 8 + # remove at: + # Cylc 9 + SuiteRunDir = 'suite_run_dir' + """The path to the workflow run directory. + + .. deprecated:: 8.0.0 + + Deprecated, use ``run_dir``. + """ + + # BACK COMPAT: suite_share_dir deprecated + # url: + # TODO + # from: + # Cylc 8 + # remove at: + # Cylc 9 + SuiteShareDir = 'suite_share_dir' + """The path to the workflow share directory. + + .. deprecated:: 8.0.0 + + Deprecated, use ``share_dir``. + """ + # Extract 'foo' from string templates '%(foo)s', avoiding '%%' escaping # ('%%(foo)s` is not a string template). @@ -124,11 +217,16 @@ def __init__( if not user: user = get_user() self.farg_templ: Dict[str, Any] = { - TMPL_WORKFLOW_NAME: workflow, - TMPL_USER_NAME: user, - TMPL_WORKFLOW_RUN_DIR: workflow_run_dir, - TMPL_WORKFLOW_SHARE_DIR: workflow_share_dir, - TMPL_DEBUG_MODE: cylc.flow.flags.verbosity > 1 + TemplateVariables.Workflow.value: workflow, + TemplateVariables.UserName.value: user, + TemplateVariables.RunDir.value: workflow_run_dir, + TemplateVariables.ShareDir.value: workflow_share_dir, + TemplateVariables.DebugMode.value: cylc.flow.flags.verbosity > 1, + # deprecated + TemplateVariables.WorkflowName.value: workflow, + TemplateVariables.SuiteName.value: workflow, + TemplateVariables.SuiteRunDir.value: workflow, + TemplateVariables.SuiteShareDir.value: workflow, } self.proc_pool = proc_pool @@ -143,38 +241,70 @@ def validate_xtrigger(label: str, fctx: SubFuncContext, fdir: str) -> None: label: xtrigger label fctx: function context fdir: function directory + Raises: - ImportError: if the function module was not found - AttributeError: if the function was not found in the xtrigger - module - ValueError: if the function is not callable - ValueError: if any string template in the function context - arguments are not present in the expected template values. + XtriggerConfigError: + * If the function module was not found. + * If the function was not found in the xtrigger module. + * If the function is not callable. + * If any string template in the function context + arguments are not present in the expected template values. + """ fname: str = fctx.func_name try: func = get_func(fname, fdir) except ImportError: - raise ImportError( - f"ERROR: xtrigger module '{fname}' not found") + raise XtriggerConfigError( + label, + fname, + f"xtrigger module '{fname}' not found", + ) except AttributeError: - raise AttributeError( - f"ERROR: '{fname}' not found in xtrigger module '{fname}'") + raise XtriggerConfigError( + label, + fname, + f"'{fname}' not found in xtrigger module '{fname}'", + ) if not callable(func): - raise ValueError( - f"ERROR: '{fname}' not callable in xtrigger module '{fname}'") + raise XtriggerConfigError( + label, + fname, + f"'{fname}' not callable in xtrigger module '{fname}'", + ) + # Check any string templates in the function arg values (note this # won't catch bad task-specific values - which are added dynamically). + template_vars = set() for argv in fctx.func_args + list(fctx.func_kwargs.values()): - try: - for match in RE_STR_TMPL.findall(argv): - if match not in ARG_VAL_TEMPLATES: - raise ValueError( - f"Illegal template in xtrigger {label}: {match}") - except TypeError: + if not isinstance(argv, str): # Not a string arg. continue + # check template variables are valid + for match in RE_STR_TMPL.findall(argv): + try: + template_vars.add(TemplateVariables(match)) + except ValueError: + raise XtriggerConfigError( + label, + fname, + f"Illegal template in xtrigger: {match}", + ) + + # check for deprecated template variables + deprecated_variables = template_vars & { + TemplateVariables.WorkflowName, + TemplateVariables.SuiteName, + TemplateVariables.SuiteRunDir, + TemplateVariables.SuiteShareDir, + } + if deprecated_variables: + LOG.warning( + f'Xtrigger "{label}" uses deprecated template variables:' + f' {", ".join(t.value for t in deprecated_variables)}' + ) + def add_trig(self, label: str, fctx: SubFuncContext, fdir: str) -> None: """Add a new xtrigger function. @@ -241,9 +371,9 @@ def get_xtrig_ctx(self, itask: TaskProxy, label: str) -> SubFuncContext: function context """ farg_templ = { - TMPL_TASK_CYCLE_POINT: str(itask.point), - TMPL_TASK_NAME: str(itask.tdef.name), - TMPL_TASK_IDENT: str(itask.identity) + TemplateVariables.CyclePoint.value: str(itask.point), + TemplateVariables.TaskName.value: str(itask.tdef.name), + TemplateVariables.TaskID.value: str(itask.identity) } farg_templ.update(self.farg_templ) ctx = deepcopy(self.functx_map[label]) diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index d9b5eaf6a57..d5894c5ff2e 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -26,9 +26,10 @@ from cylc.flow.cycling import loader from cylc.flow.cycling.loader import INTEGER_CYCLING_TYPE, ISO8601_CYCLING_TYPE from cylc.flow.exceptions import ( - WorkflowConfigError, PointParsingError, - UserInputError + UserInputError, + WorkflowConfigError, + XtriggerConfigError, ) from cylc.flow.workflow_files import WorkflowFiles from cylc.flow.wallclock import get_utc_mode, set_utc_mode @@ -122,7 +123,7 @@ def test_xfunction_import_error(self, mock_glbl_cfg, tmp_path): R1 = '@oopsie => qux' """ flow_file.write_text(flow_config) - with pytest.raises(ImportError) as excinfo: + with pytest.raises(XtriggerConfigError) as excinfo: WorkflowConfig( workflow="caiman_workflow", fpath=flow_file, @@ -155,7 +156,7 @@ def test_xfunction_attribute_error(self, mock_glbl_cfg, tmp_path): R1 = '@oopsie => qux' """ flow_file.write_text(flow_config) - with pytest.raises(AttributeError) as excinfo: + with pytest.raises(XtriggerConfigError) as excinfo: WorkflowConfig(workflow="capybara_workflow", fpath=flow_file, options=Mock(spec=[])) assert "not found" in str(excinfo.value) @@ -185,7 +186,7 @@ def test_xfunction_not_callable(self, mock_glbl_cfg, tmp_path): R1 = '@oopsie => qux' """ flow_file.write_text(flow_config) - with pytest.raises(ValueError) as excinfo: + with pytest.raises(XtriggerConfigError) as excinfo: WorkflowConfig( workflow="workflow_with_not_callable", fpath=flow_file, diff --git a/tests/unit/test_xtrigger_mgr.py b/tests/unit/test_xtrigger_mgr.py index 1493f8cdd5a..ec547364f0c 100644 --- a/tests/unit/test_xtrigger_mgr.py +++ b/tests/unit/test_xtrigger_mgr.py @@ -14,9 +14,12 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import logging import pytest +from cylc.flow import CYLC_LOG from cylc.flow.cycling.iso8601 import ISO8601Point, ISO8601Sequence, init +from cylc.flow.exceptions import XtriggerConfigError from cylc.flow.subprocctx import SubFuncContext from cylc.flow.task_proxy import TaskProxy from cylc.flow.taskdef import TaskDef @@ -82,10 +85,25 @@ def test_add_xtrigger_with_unknown_params(xtrigger_mgr): func_args=[1, "name", "%(what_is_this)s"], func_kwargs={"location": "soweto"} ) - with pytest.raises(ValueError): + with pytest.raises(XtriggerConfigError): xtrigger_mgr.add_trig("xtrig", xtrig, 'fdir') +def test_add_xtrigger_with_deprecated_params(xtrigger_mgr, caplog): + """It should flag deprecated template variables.""" + xtrig = SubFuncContext( + label="echo", + func_name="echo", + func_args=[1, "name", "%(suite_name)s"], + func_kwargs={"location": "soweto"} + ) + caplog.set_level(logging.WARNING, CYLC_LOG) + xtrigger_mgr.add_trig("xtrig", xtrig, 'fdir') + assert caplog.messages == [ + 'Xtrigger "xtrig" uses deprecated template variables: suite_name' + ] + + def test_load_xtrigger_for_restart(xtrigger_mgr): """Test loading an xtrigger for restart.