Ensure mail event names are validated in config

Moved validation of workflow event names to parsec

Author: MetRonnie

changes.d/6838.fix.md

@@ -1 +1 @@
-Workflow and task `handler events` names are now validated. Outdated Cylc 7 workflow event names are automatically upgraded.
+Workflow and task `handler events` and `mail events` names are now validated. Outdated Cylc 7 workflow event names are automatically upgraded.

cylc/flow/cfgspec/globalcfg.py

@@ -15,39 +15,55 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 """Cylc site and user configuration file spec."""
 
+from contextlib import suppress
 import os
 from pathlib import Path
 from sys import stderr
-from textwrap import dedent, indent
-from typing import List, Optional, Tuple, Any, Union
+from textwrap import (
+    dedent,
+    indent,
+)
+from typing import (
+    Any,
+    Dict,
+    List,
+    Optional,
+    Tuple,
+    Union,
+)
 
-from contextlib import suppress
 from packaging.version import Version
 
-from cylc.flow import LOG
-from cylc.flow import __version__ as CYLC_VERSION
-from cylc.flow.platforms import validate_platforms
+from cylc.flow import (
+    LOG,
+    __version__ as CYLC_VERSION,
+)
 from cylc.flow.exceptions import GlobalConfigError
 from cylc.flow.hostuserutil import get_user_home
 from cylc.flow.network.client_factory import CommsMeth
-from cylc.flow.pathutil import SYMLINKABLE_LOCATIONS
 from cylc.flow.parsec.config import (
     ConfigNode as Conf,
     ParsecConfig,
 )
 from cylc.flow.parsec.exceptions import (
-    ParsecError,
     ItemNotFoundError,
+    ParsecError,
     ValidationError,
 )
 from cylc.flow.parsec.upgrade import upgrader
-from cylc.flow.parsec.util import printcfg, expand_many_section
+from cylc.flow.parsec.util import (
+    expand_many_section,
+    printcfg,
+)
 from cylc.flow.parsec.validate import (
     CylcConfigValidator as VDR,
     DurationFloat,
     Range,
     cylc_config_validate,
 )
+from cylc.flow.pathutil import SYMLINKABLE_LOCATIONS
+from cylc.flow.platforms import validate_platforms
+from cylc.flow.workflow_events import WorkflowEventHandler
 
 
 PLATFORM_REGEX_TEXT = '''
@@ -150,7 +166,7 @@
 Configure the workflow event handling system.
 '''
 
-EVENTS_SETTINGS = {  # workflow events
+EVENTS_SETTINGS: Dict[str, Union[str, Dict[str, Any]]] = {  # workflow events
     'handlers': '''
         Configure :term:`event handlers` that run when certain workflow
         events occur.
@@ -171,17 +187,26 @@
 
            :ref:`user_guide.scheduler.workflow_events`
     ''',
-    'handler events': '''
-        Specify the events for which workflow event handlers should be invoked.
+    'handler events': {
+        'desc': '''
+            Specify the events for which workflow event handlers should be
+            invoked.
 
-        .. seealso::
+            .. seealso::
 
-           :ref:`user_guide.scheduler.workflow_events`
-    ''',
-    'mail events': '''
-        Specify the workflow events for which notification emails should
-        be sent.
-    ''',
+               :ref:`user_guide.scheduler.workflow_events`
+        ''',
+        'options': WorkflowEventHandler.EVENTS.copy(),
+        'depr_options': WorkflowEventHandler.EVENTS_DEPRECATED.copy(),
+    },
+    'mail events': {
+        'desc': '''
+            Specify the workflow events for which notification emails should
+            be sent.
+        ''',
+        'options': WorkflowEventHandler.EVENTS.copy(),
+        'depr_options': WorkflowEventHandler.EVENTS_DEPRECATED.copy(),
+    },
     'startup handlers': f'''
         Handlers to run at scheduler startup.
 
@@ -1038,7 +1063,12 @@ def default_for(
 
         with Conf('events',
                   desc=default_for(EVENTS_DESCR, '[scheduler][events]')):
-            for item, desc in EVENTS_SETTINGS.items():
+            for item, val in EVENTS_SETTINGS.items():
+                if isinstance(val, dict):
+                    desc: str = val.pop('desc', '')
+                else:
+                    desc = val
+                    val = {}
                 desc = default_for(desc, f"[scheduler][events]{item}")
                 vdr_type = VDR.V_STRING_LIST
                 default: Any = Conf.UNSET
@@ -1058,7 +1088,7 @@ def default_for(
                         default = DurationFloat(300)
                     else:
                         default = None
-                Conf(item, vdr_type, default, desc=desc)
+                Conf(item, vdr_type, default, desc=desc, **val)
 
         with Conf('mail', desc=(
             default_for(MAIL_DESCR, "[scheduler][mail]", section=True)

cylc/flow/cfgspec/workflow.py

@@ -411,9 +411,15 @@ def get_script_common_text(this: str, example: Optional[str] = None):
                     )
                 ))
 
-        with Conf('events',
-                  desc=global_default(EVENTS_DESCR, '[scheduler][events]')):
-            for item, desc in EVENTS_SETTINGS.items():
+        with Conf(
+            'events', desc=global_default(EVENTS_DESCR, '[scheduler][events]')
+        ):
+            for item, val in EVENTS_SETTINGS.items():
+                if isinstance(val, dict):
+                    desc: str = val.pop('desc', '')
+                else:
+                    desc = val
+                    val = {}
                 desc = global_default(desc, f"[scheduler][events]{item}")
                 vdr_type = VDR.V_STRING_LIST
                 default: Any = Conf.UNSET
@@ -444,7 +450,7 @@ def get_script_common_text(this: str, example: Optional[str] = None):
                     vdr_type = VDR.V_BOOLEAN
                 elif item.endswith("timeout"):
                     vdr_type = VDR.V_INTERVAL
-                Conf(item, vdr_type, default, desc=desc)
+                Conf(item, vdr_type, default, desc=desc, **val)
 
             Conf('expected task failures', VDR.V_STRING_LIST, desc='''
                 (For Cylc developers writing a functional tests only)

cylc/flow/config.py

@@ -542,7 +542,7 @@ def __init__(
             self.cfg['scheduling']['special tasks'][s_type] = result
 
         self.process_config_env()
-        self._check_and_upg_wflow_handler_events()
+        self._upg_wflow_event_names()
 
         self.mem_log("config.py: before load_graph()")
         self.load_graph()
@@ -560,7 +560,7 @@ def __init__(
         self.configure_workflow_state_polling_tasks()
 
         for taskdef in self.taskdefs.values():
-            self._check_task_handler_events(taskdef)
+            self._check_task_event_names(taskdef)
             self._check_task_event_handlers(taskdef)
         self._check_special_tasks()  # adds to self.implicit_tasks
         self._check_explicit_cycling()
@@ -1745,23 +1745,24 @@ def process_config_env(self):
                 ]
             )
 
-    def _check_task_handler_events(self, taskdef: 'TaskDef') -> None:
-        """Validate task handler event names."""
-        handler_events: Optional[List[str]] = taskdef.rtconfig['events'][
-            'handler events'
-        ]
-        if not handler_events:
-            return
-        invalid = set(handler_events).difference(
-            TaskEventsManager.STD_EVENTS,
-            taskdef.rtconfig['outputs'],
-        )
-        if invalid:
-            raise WorkflowConfigError(
-                "Invalid event name(s) for "
-                f"[runtime][{taskdef.name}][events]handler events: "
-                + ', '.join(sorted(invalid))
+    def _check_task_event_names(self, taskdef: 'TaskDef') -> None:
+        """Validate task handler/mail event names."""
+        for setting in ('handler events', 'mail events'):
+            event_names: Optional[List[str]] = taskdef.rtconfig['events'][
+                setting
+            ]
+            if not event_names:
+                continue
+            invalid = set(event_names).difference(
+                TaskEventsManager.STD_EVENTS,
+                taskdef.rtconfig['outputs'],
             )
+            if invalid:
+                raise WorkflowConfigError(
+                    "Invalid event name(s) for "
+                    f"[runtime][{taskdef.name}][events]{setting}: "
+                    + ', '.join(sorted(invalid))
+                )
 
     def _check_task_event_handlers(self, taskdef: 'TaskDef') -> None:
         """Check custom event handler templates can be expanded.
@@ -2726,32 +2727,23 @@ def upgrade_clock_triggers(self):
             for seq in taskdef.sequences:
                 taskdef.add_xtrig_label(label, seq)
 
-    def _check_and_upg_wflow_handler_events(self) -> None:
-        """Validate workflow handler events and upgrade any Cylc 7 event names.
-        """
-        handler_events: Optional[List[str]] = self.cfg['scheduler']['events'][
-            'handler events'
-        ]
-        if not handler_events:
-            return
-        upgraded: Dict[str, str] = {}
-        for i, event in enumerate(handler_events):
-            if event in WorkflowEventHandler.EVENTS_DEPRECATED:
-                handler_events[i] = upgraded[event] = (
-                    WorkflowEventHandler.EVENTS_DEPRECATED[event]
-                )
-            elif event not in WorkflowEventHandler.EVENTS:
-                valid = WorkflowEventHandler.EVENTS.copy()
-                if cylc.flow.flags.cylc7_back_compat:
-                    valid += WorkflowEventHandler.EVENTS_DEPRECATED
-                raise WorkflowConfigError(
-                    f"Invalid workflow handler event '{event}'. "
-                    "[scheduler][events][handler events] must be one of: "
-                    + ', '.join(valid)
+    def _upg_wflow_event_names(self) -> None:
+        """Upgrade any Cylc 7 workflow handler/mail events names."""
+        for setting in ('handler events', 'mail events'):
+            event_names: Optional[List[str]] = self.cfg['scheduler']['events'][
+                setting
+            ]
+            if not event_names:
+                continue
+            upgraded: Dict[str, str] = {}
+            for i, event in enumerate(event_names):
+                if event in WorkflowEventHandler.EVENTS_DEPRECATED:
+                    event_names[i] = upgraded[event] = (
+                        WorkflowEventHandler.EVENTS_DEPRECATED[event]
+                    )
+            if upgraded and not cylc.flow.flags.cylc7_back_compat:
+                LOG.warning(
+                    f"{upgrader.DEPR_MSG}\n"
+                    f" * (8.0.0) [scheduler][events][{setting}] "
+                    + ', '.join(f'{k} -> {v}' for k, v in upgraded.items())
                 )
-        if upgraded and not cylc.flow.flags.cylc7_back_compat:
-            LOG.warning(
-                f"{upgrader.DEPR_MSG}\n"
-                " * (8.0.0) [scheduler][events][handler events] "
-                + ', '.join(f'{k} -> {v}' for k, v in upgraded.items())
-            )

cylc/flow/parsec/config.py

@@ -246,6 +246,9 @@ class ConfigNode(ContextNode):
         options:
             List of possible options.
             TODO: allow this to be a dict with help info
+        depr_options:
+            List of deprecated options. These are not displayed in the docs
+            but are used for backwards compatibility.
         default:
             The default value.
         desc:
@@ -273,7 +276,13 @@ class ConfigNode(ContextNode):
     UNSET = '*value unset*'
 
     __slots__ = ContextNode.__slots__ + (
-        'vdr', 'options', 'default', 'desc', 'display_name', 'meta'
+        'vdr',
+        'options',
+        'depr_options',
+        'default',
+        'desc',
+        'display_name',
+        'meta',
     )
 
     def __init__(
@@ -283,7 +292,8 @@ def __init__(
         default: object = UNSET,
         options: Optional[list] = None,
         desc: Optional[str] = None,
-        meta: Optional['ConfigNode'] = None
+        meta: Optional['ConfigNode'] = None,
+        depr_options: Optional[list] = None,
     ):
         display_name = name
         if name.startswith('<'):
@@ -306,7 +316,8 @@ def __init__(
         self.display_name = display_name
         self.vdr = vdr
         self.default = default
-        self.options = options
+        self.options = options or []
+        self.depr_options = depr_options or []
         self.desc = dedent(desc).strip() if desc else None
         self.meta = meta
 

cylc/flow/parsec/validate.py

@@ -230,13 +230,19 @@ def validate(self, cfg_root, spec_root):
                     # Item is value, coerce according to value type
                     cfg[key] = self.coercers[specval.vdr](value, keys + [key])
                     if specval.options:
-                        voptions = specval.options
-                        if (isinstance(cfg[key], list) and
-                                any(val not in voptions for val in cfg[key]) or
-                                not isinstance(cfg[key], list) and
-                                cfg[key] not in voptions):
+                        voptions = {*specval.options, *specval.depr_options}
+                        if isinstance(cfg[key], list):
+                            bad = [
+                                str(i) for i in cfg[key] if i not in voptions
+                            ]
+                            if bad:
+                                raise IllegalValueError(
+                                    'option', [*keys, key], ', '.join(bad)
+                                )
+                        elif cfg[key] not in voptions:
                             raise IllegalValueError(
-                                'option', keys + [key], cfg[key])
+                                'option', [*keys, key], cfg[key]
+                            )
 
     __call__ = validate
 

tests/integration/test_workflow_events.py

@@ -202,7 +202,7 @@ def mock_run_event_handlers(self, event, reason=""):
     # Configure a long-running shutdown handler.
     schd = test_scheduler({
         'shutdown handlers': 'sleep 10; echo',
-        'mail events': [],
+        'mail events': '',
     })
 
     # Set a low process pool timeout value.