Skip to content

Commit 0ff968b

Browse files
config: turn invalid workflow event errors into warnings
* Turns out a lot of workflows have invalid mail/handlert pu events. * Turn this hard error into a soft warning to make migration easier. * Also include the list of valid task events in the config docs. Closes #6870
1 parent df72a56 commit 0ff968b

File tree

6 files changed

+88
-17
lines changed

6 files changed

+88
-17
lines changed

changes.d/6902.feat.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Invalid workflow events in the `mail events` or `handler events` configurations will result in warnings rather than errors.

cylc/flow/cfgspec/globalcfg.py

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
)
6464
from cylc.flow.pathutil import SYMLINKABLE_LOCATIONS
6565
from cylc.flow.platforms import validate_platforms
66+
from cylc.flow.task_events_mgr import TaskEventsManager as TEM
6667
from cylc.flow.workflow_events import WorkflowEventHandler
6768

6869

@@ -200,6 +201,7 @@
200201
''',
201202
'options': WorkflowEventHandler.EVENTS.copy(),
202203
'depr_options': WorkflowEventHandler.EVENTS_DEPRECATED.copy(),
204+
'warn_options': True,
203205
},
204206
'mail events': {
205207
'desc': '''
@@ -212,6 +214,7 @@
212214
''',
213215
'options': WorkflowEventHandler.EVENTS.copy(),
214216
'depr_options': WorkflowEventHandler.EVENTS_DEPRECATED.copy(),
217+
'warn_options': True,
215218
},
216219
'startup handlers': f'''
217220
Handlers to run at scheduler startup.
@@ -590,11 +593,13 @@
590593
echo %(event)s occurred in %(workflow)s >> my-log-file
591594
592595
''',
593-
'handler events': '''
596+
'handler events': f'''
597+
:Options: ``{"``, ``".join(TEM.STD_EVENTS)}`` & any custom event
598+
594599
A list of events for which :cylc:conf:`[..]handlers` are run.
595600
596-
See :ref:`user_guide.runtime.task_event_handling.list` for valid
597-
events.
601+
See :ref:`user_guide.runtime.task_event_handling.list` for more
602+
information on task events.
598603
599604
Example::
600605
@@ -611,11 +616,13 @@
611616
612617
PT10S, PT1M, PT5M
613618
''',
614-
'mail events': '''
619+
'mail events': f'''
620+
:Options: ``{"``, ``".join(TEM.STD_EVENTS)}`` & any custom event
621+
615622
A list of events for which notification emails should be sent.
616623
617-
See :ref:`user_guide.runtime.task_event_handling.list` for valid
618-
events.
624+
See :ref:`user_guide.runtime.task_event_handling.list` for more
625+
information on task events.
619626
620627
Example::
621628
@@ -676,15 +683,36 @@ def comma_sep_section_note(version_changed: str = '') -> str:
676683

677684

678685
def short_descr(text: str) -> str:
679-
"""Get dedented one-paragraph description from long description."""
680-
return dedent(text).split('\n\n', 1)[0]
686+
r"""Get dedented one-paragraph description from long description.
687+
688+
Examples:
689+
>>> short_descr('foo\n\nbar')
690+
'foo'
691+
692+
>>> short_descr(':Field: Value\n\nfoo\n\nbar')
693+
':Field: Value\n\nfoo'
694+
695+
"""
696+
lines = []
697+
for line in dedent(text).splitlines():
698+
if not line:
699+
continue
700+
elif line.startswith(':'):
701+
lines.append(line)
702+
else:
703+
lines.append(line)
704+
break
705+
return '\n\n'.join(lines)
681706

682707

683708
def default_for(
684709
text: str, config_path: str, section: bool = False
685710
) -> str:
686-
"""Get dedented short description and insert a 'Default(s) For' directive
687-
that links to this config item's flow.cylc counterpart."""
711+
"""Return a ":Default For: field for this config.
712+
713+
Get dedented short description and insert a 'Default(s) For' field
714+
that links to this config item's flow.cylc counterpart.
715+
"""
688716
directive = f":Default{'s' if section else ''} For:"
689717
return (
690718
f"{directive} :cylc:conf:`flow.cylc{config_path}`.\n\n"

cylc/flow/parsec/config.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,10 @@ class ConfigNode(ContextNode):
249249
depr_options:
250250
List of deprecated options. These are not displayed in the docs
251251
but are used for backwards compatibility.
252+
warn_options:
253+
If True, parsec will warn if invalid options are present rather
254+
than raising an exception. Any invalid options will be stripped
255+
from the config.
252256
default:
253257
The default value.
254258
desc:
@@ -279,6 +283,7 @@ class ConfigNode(ContextNode):
279283
'vdr',
280284
'options',
281285
'depr_options',
286+
'warn_options',
282287
'default',
283288
'desc',
284289
'display_name',
@@ -294,6 +299,7 @@ def __init__(
294299
desc: Optional[str] = None,
295300
meta: Optional['ConfigNode'] = None,
296301
depr_options: Optional[list] = None,
302+
warn_options: bool = False,
297303
):
298304
display_name = name
299305
if name.startswith('<'):
@@ -318,6 +324,7 @@ def __init__(
318324
self.default = default
319325
self.options = options or []
320326
self.depr_options = depr_options or []
327+
self.warn_options = warn_options
321328
self.desc = dedent(desc).strip() if desc else None
322329
self.meta = meta
323330

cylc/flow/parsec/validate.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
from metomi.isodatetime.parsers import TimePointParser, DurationParser
3434
from metomi.isodatetime.exceptions import IsodatetimeError, ISO8601SyntaxError
3535

36+
from cylc.flow import LOG
3637
from cylc.flow.parsec.exceptions import (
3738
ListValueError, IllegalValueError, IllegalItemError)
3839
from cylc.flow.subprocctx import SubFuncContext
@@ -236,9 +237,19 @@ def validate(self, cfg_root, spec_root):
236237
str(i) for i in cfg[key] if i not in voptions
237238
]
238239
if bad:
239-
raise IllegalValueError(
240+
exc = IllegalValueError(
240241
'option', [*keys, key], ', '.join(bad)
241242
)
243+
if specval.warn_options:
244+
LOG.warning(
245+
f'{exc}'
246+
'\nInvalid items have been removed'
247+
)
248+
cfg[key] = [
249+
x for x in cfg[key] if x not in bad
250+
]
251+
else:
252+
raise exc
242253
elif cfg[key] not in voptions:
243254
raise IllegalValueError(
244255
'option', [*keys, key], cfg[key]

tests/unit/parsec/test_validate.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@
1515
# along with this program. If not, see <http://www.gnu.org/licenses/>.
1616
"""Unit Tests for cylc.flow.parsec.validate.ParsecValidator.coerce methods."""
1717

18+
import logging
1819
from typing import List
1920

21+
from cylc.flow import CYLC_LOG
2022
import pytest
2123
from pytest import approx, param
2224

@@ -253,6 +255,30 @@ def test_parsec_validator_invalid_key_with_many_invalid_values(
253255
assert parsec_validator is not None
254256

255257

258+
def test_parsec_validator_warn_options(caplog):
259+
"""Test the "warn_options" option.
260+
261+
This should turn invalid option errors into warnings.
262+
"""
263+
with Conf('base') as spec:
264+
Conf(
265+
'foo',
266+
VDR.V_STRING_LIST,
267+
default=1,
268+
options=['a', 'b'],
269+
warn_options=True,
270+
)
271+
272+
parsec_validator = ParsecValidator()
273+
caplog.set_level(logging.WARNING, CYLC_LOG)
274+
parsec_validator.validate({'foo': 'b, c'}, spec)
275+
276+
# there should be one (and only one) warning
277+
assert caplog.messages == [
278+
'(type=option) foo = c\nInvalid items have been removed'
279+
]
280+
281+
256282
def test_parsec_validator_invalid_key_with_many_1(sample_spec):
257283
parsec_validator = ParsecValidator()
258284
cfg = OrderedDictWithDefaults()

tests/unit/test_config.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
XtriggerConfigError,
5353
)
5454
from cylc.flow.parsec.exceptions import (
55-
IllegalValueError,
5655
Jinja2Error,
5756
)
5857
from cylc.flow.scheduler_cli import RunOptions
@@ -1776,8 +1775,8 @@ def test_upg_wflow_event_names(back_compat, tmp_flow_config, log_filter):
17761775

17771776

17781777
@pytest.mark.parametrize('item', ['handler events', 'mail events'])
1779-
def test_val_wflow_event_names(item, tmp_flow_config):
1780-
"""Any invalid workflow handler/mail events raise an error."""
1778+
def test_val_wflow_event_names(item, tmp_flow_config, log_filter):
1779+
"""Any invalid workflow handler/mail events raise a warning."""
17811780
flow_file = tmp_flow_config('foo', f"""
17821781
[scheduler]
17831782
allow implicit tasks = true
@@ -1787,9 +1786,8 @@ def test_val_wflow_event_names(item, tmp_flow_config):
17871786
[[graph]]
17881787
R1 = foo
17891788
""")
1790-
with pytest.raises(IllegalValueError) as ex_info:
1791-
WorkflowConfig('foo', str(flow_file), ValidateOptions())
1792-
assert f"{item} = badger, alpaca" in str(ex_info.value)
1789+
WorkflowConfig('foo', str(flow_file), ValidateOptions())
1790+
assert log_filter(contains=f"{item} = badger, alpaca")
17931791

17941792

17951793
@pytest.mark.parametrize('item', ['handler events', 'mail events'])

0 commit comments

Comments
 (0)