Merge pull request #6902 from oliver-sanders/6870

config: turn invalid workflow event errors into warnings

Author: ChrisPaulBennett

changes.d/6902.feat.md

@@ -0,0 +1 @@
+Invalid workflow events in the `mail events` or `handler events` configurations will result in warnings rather than errors.

cylc/flow/cfgspec/globalcfg.py

@@ -63,6 +63,7 @@
 )
 from cylc.flow.pathutil import SYMLINKABLE_LOCATIONS
 from cylc.flow.platforms import validate_platforms
+from cylc.flow.task_events_mgr import TaskEventsManager as TEM
 from cylc.flow.workflow_events import WorkflowEventHandler
 
 
@@ -200,6 +201,7 @@
         ''',
         'options': WorkflowEventHandler.EVENTS.copy(),
         'depr_options': WorkflowEventHandler.EVENTS_DEPRECATED.copy(),
+        'warn_options': True,
     },
     'mail events': {
         'desc': '''
@@ -212,6 +214,7 @@
         ''',
         'options': WorkflowEventHandler.EVENTS.copy(),
         'depr_options': WorkflowEventHandler.EVENTS_DEPRECATED.copy(),
+        'warn_options': True,
     },
     'startup handlers': f'''
         Handlers to run at scheduler startup.
@@ -590,11 +593,13 @@
            echo %(event)s occurred in %(workflow)s >> my-log-file
 
     ''',
-    'handler events': '''
+    'handler events': f'''
+        :Options: ``{"``, ``".join(TEM.STD_EVENTS)}`` & any custom event
+
         A list of events for which :cylc:conf:`[..]handlers` are run.
 
-        See :ref:`user_guide.runtime.task_event_handling.list` for valid
-        events.
+        See :ref:`user_guide.runtime.task_event_handling.list` for more
+        information on task events.
 
         Example::
 
@@ -611,11 +616,13 @@
 
            PT10S, PT1M, PT5M
     ''',
-    'mail events': '''
+    'mail events': f'''
+        :Options: ``{"``, ``".join(TEM.STD_EVENTS)}`` & any custom event
+
         A list of events for which notification emails should be sent.
 
-        See :ref:`user_guide.runtime.task_event_handling.list` for valid
-        events.
+        See :ref:`user_guide.runtime.task_event_handling.list` for more
+        information on task events.
 
         Example::
 
@@ -676,15 +683,36 @@ def comma_sep_section_note(version_changed: str = '') -> str:
 
 
 def short_descr(text: str) -> str:
-    """Get dedented one-paragraph description from long description."""
-    return dedent(text).split('\n\n', 1)[0]
+    r"""Get dedented one-paragraph description from long description.
+
+    Examples:
+        >>> short_descr('foo\n\nbar')
+        'foo'
+
+        >>> short_descr(':Field: Value\n\nfoo\n\nbar')
+        ':Field: Value\n\nfoo'
+
+    """
+    lines = []
+    for line in dedent(text).splitlines():
+        if not line:
+            continue
+        elif line.startswith(':'):
+            lines.append(line)
+        else:
+            lines.append(line)
+            break
+    return '\n\n'.join(lines)
 
 
 def default_for(
     text: str, config_path: str, section: bool = False
 ) -> str:
-    """Get dedented short description and insert a 'Default(s) For' directive
-    that links to this config item's flow.cylc counterpart."""
+    """Return a ":Default For: field for this config.
+
+    Get dedented short description and insert a 'Default(s) For' field
+    that links to this config item's flow.cylc counterpart.
+    """
     directive = f":Default{'s' if section else ''} For:"
     return (
         f"{directive} :cylc:conf:`flow.cylc{config_path}`.\n\n"

cylc/flow/parsec/config.py

@@ -249,6 +249,10 @@ class ConfigNode(ContextNode):
         depr_options:
             List of deprecated options. These are not displayed in the docs
             but are used for backwards compatibility.
+        warn_options:
+            If True, parsec will warn if invalid options are present rather
+            than raising an exception. Any invalid options will be stripped
+            from the config.
         default:
             The default value.
         desc:
@@ -279,6 +283,7 @@ class ConfigNode(ContextNode):
         'vdr',
         'options',
         'depr_options',
+        'warn_options',
         'default',
         'desc',
         'display_name',
@@ -294,6 +299,7 @@ def __init__(
         desc: Optional[str] = None,
         meta: Optional['ConfigNode'] = None,
         depr_options: Optional[list] = None,
+        warn_options: bool = False,
     ):
         display_name = name
         if name.startswith('<'):
@@ -318,6 +324,7 @@ def __init__(
         self.default = default
         self.options = options or []
         self.depr_options = depr_options or []
+        self.warn_options = warn_options
         self.desc = dedent(desc).strip() if desc else None
         self.meta = meta
 

cylc/flow/parsec/validate.py

@@ -33,6 +33,7 @@
 from metomi.isodatetime.parsers import TimePointParser, DurationParser
 from metomi.isodatetime.exceptions import IsodatetimeError, ISO8601SyntaxError
 
+from cylc.flow import LOG
 from cylc.flow.parsec.exceptions import (
     ListValueError, IllegalValueError, IllegalItemError)
 from cylc.flow.subprocctx import SubFuncContext
@@ -236,9 +237,19 @@ def validate(self, cfg_root, spec_root):
                                 str(i) for i in cfg[key] if i not in voptions
                             ]
                             if bad:
-                                raise IllegalValueError(
+                                exc = IllegalValueError(
                                     'option', [*keys, key], ', '.join(bad)
                                 )
+                                if specval.warn_options:
+                                    LOG.warning(
+                                        f'{exc}'
+                                        '\nInvalid items have been removed'
+                                    )
+                                    cfg[key] = [
+                                        x for x in cfg[key] if x not in bad
+                                    ]
+                                else:
+                                    raise exc
                         elif cfg[key] not in voptions:
                             raise IllegalValueError(
                                 'option', [*keys, key], cfg[key]

tests/unit/parsec/test_validate.py

@@ -15,8 +15,10 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 """Unit Tests for cylc.flow.parsec.validate.ParsecValidator.coerce methods."""
 
+import logging
 from typing import List
 
+from cylc.flow import CYLC_LOG
 import pytest
 from pytest import approx, param
 
@@ -253,6 +255,30 @@ def test_parsec_validator_invalid_key_with_many_invalid_values(
         assert parsec_validator is not None
 
 
+def test_parsec_validator_warn_options(caplog):
+    """Test the "warn_options" option.
+
+    This should turn invalid option errors into warnings.
+    """
+    with Conf('base') as spec:
+        Conf(
+            'foo',
+            VDR.V_STRING_LIST,
+            default=1,
+            options=['a', 'b'],
+            warn_options=True,
+        )
+
+    parsec_validator = ParsecValidator()
+    caplog.set_level(logging.WARNING, CYLC_LOG)
+    parsec_validator.validate({'foo': 'b, c'}, spec)
+
+    # there should be one (and only one) warning
+    assert caplog.messages == [
+        '(type=option) foo = c\nInvalid items have been removed'
+    ]
+
+
 def test_parsec_validator_invalid_key_with_many_1(sample_spec):
     parsec_validator = ParsecValidator()
     cfg = OrderedDictWithDefaults()

tests/unit/test_config.py

@@ -52,7 +52,6 @@
     XtriggerConfigError,
 )
 from cylc.flow.parsec.exceptions import (
-    IllegalValueError,
     Jinja2Error,
 )
 from cylc.flow.scheduler_cli import RunOptions
@@ -1776,8 +1775,8 @@ def test_upg_wflow_event_names(back_compat, tmp_flow_config, log_filter):
 
 
 @pytest.mark.parametrize('item', ['handler events', 'mail events'])
-def test_val_wflow_event_names(item, tmp_flow_config):
-    """Any invalid workflow handler/mail events raise an error."""
+def test_val_wflow_event_names(item, tmp_flow_config, log_filter):
+    """Any invalid workflow handler/mail events raise a warning."""
     flow_file = tmp_flow_config('foo', f"""
         [scheduler]
             allow implicit tasks = true
@@ -1787,9 +1786,8 @@ def test_val_wflow_event_names(item, tmp_flow_config):
             [[graph]]
                 R1 = foo
     """)
-    with pytest.raises(IllegalValueError) as ex_info:
-        WorkflowConfig('foo', str(flow_file), ValidateOptions())
-    assert f"{item} = badger, alpaca" in str(ex_info.value)
+    WorkflowConfig('foo', str(flow_file), ValidateOptions())
+    assert log_filter(contains=f"{item} = badger, alpaca")
 
 
 @pytest.mark.parametrize('item', ['handler events', 'mail events'])