Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail by default if implicit/naked tasks present #4109

Merged
merged 11 commits into from
Mar 11, 2021
9 changes: 7 additions & 2 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ 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.0a3 (<span actions:bind='release-date'>2020-08?</span>)__
## __cylc-8.0b0 (<span actions:bind='release-date'>2020-08?</span>)__

Fourth alpha release of Cylc 8.
First beta release of Cylc 8.

(See note on cylc-8 backward-incompatible changes, above)

Expand All @@ -70,6 +70,11 @@ queueing logic centralized.
how workflows are restarted
([#4040](https://github.com/cylc/cylc-flow/pull/4040)).

"Implicit"/"naked" tasks (tasks that do not have an explicit definition in
`flow.cylc[runtime]`) are now disallowed by default
([#4109](https://github.com/cylc/cylc-flow/pull/4109)). You can allow them by
setting `flow.cylc[scheduler]allow implicit tasks` to `True`.

### Enhancements

[#4071](https://github.com/cylc/cylc-flow/pull/4071) - Cylc reinstall command
Expand Down
15 changes: 13 additions & 2 deletions cylc/flow/cfgspec/suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,17 @@
with Conf('scheduler'):
Conf('UTC mode', VDR.V_BOOLEAN)

Conf('allow implicit tasks', VDR.V_BOOLEAN, default=False, desc='''
:term:`Implicit tasks <implicit task>` are tasks without explicit
runtime definitions in :cylc:conf:`flow.cylc[runtime]`. By default,
these are not allowed, as often they happen to be typos. However,
this setting can be set to ``True`` to allow implict tasks.
It is recommended to set this to ``True`` if required during
development/prototyping of a workflow graph, but set it to
``False`` after finishing the :cylc:conf:`flow.cylc[runtime]`
section.
''')

Conf('install', VDR.V_STRING_LIST, desc='''
Configure the directories and files to be included in the remote
file installation.
Expand Down Expand Up @@ -651,7 +662,7 @@

Names may not contain colons (which would preclude use of
directory paths involving the registration name in ``$PATH``
variables). They may not contain the "." character (it will be
variables). They may not contain the ``.`` character (it will be
interpreted as the namespace hierarchy delimiter, separating
groups and names -huh?).

Expand All @@ -663,7 +674,7 @@
If multiple names are listed the subsequent settings apply to
each.

All namespaces inherit initially from *root*, which can be
All namespaces inherit initially from ``root``, which can be
explicitly configured to provide or override default settings for
all tasks in the suite.
'''):
Expand Down
56 changes: 31 additions & 25 deletions cylc/flow/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import os
import re
import traceback
from typing import Set

from metomi.isodatetime.data import Calendar
from metomi.isodatetime.parsers import DurationParser
Expand Down Expand Up @@ -161,7 +162,7 @@ def __init__(
self.share_dir = share_dir or get_suite_run_share_dir(self.suite)
self.work_dir = work_dir or get_suite_run_work_dir(self.suite)
self.options = options
self.naked_tasks = []
self.implicit_tasks: Set[str] = set()
self.edges = {}
self.taskdefs = {}
self.initial_point = None
Expand Down Expand Up @@ -461,21 +462,27 @@ def __init__(
self.configure_suite_state_polling_tasks()

self._check_task_event_handlers()
self._check_special_tasks() # adds to self.naked_tasks
self._check_special_tasks() # adds to self.implicit_tasks
self._check_explicit_cycling()

# Warn or abort (if --strict) if naked tasks (no runtime
# section) are found in graph or queue config.
if len(self.naked_tasks) > 0:
if self._is_validate(is_strict=True) or cylc.flow.flags.verbose:
err_msg = ('naked tasks detected (no entry'
' under [runtime]):')
for ndt in self.naked_tasks:
err_msg += '\n+\t' + str(ndt)
LOG.warning(err_msg)
if self._is_validate(is_strict=True):
# Warn or abort if implicit tasks are found in graph or queue config
if self.implicit_tasks:
print_limit = 10
implicit_tasks_str = ', '.join(
list(self.implicit_tasks)[:print_limit])
num = len(self.implicit_tasks)
if num > print_limit:
implicit_tasks_str = f"{implicit_tasks_str} and {num} others"
err_msg = (
"implicit tasks detected (no entry under [runtime]): "
f"{implicit_tasks_str}")
if self.cfg['scheduler']['allow implicit tasks']:
LOG.debug(err_msg)
else:
raise SuiteConfigError(
'strict validation fails naked tasks')
f"{err_msg}\n\n"
"To allow implicit tasks, use "
"'flow.cylc[scheduler]allow implicit tasks'")

# Check that external trigger messages are only used once (they have to
# be discarded immediately to avoid triggering the next instance of the
Expand Down Expand Up @@ -640,7 +647,7 @@ def __init__(
cfg['meta']['URL'] = RE_TASK_NAME_VAR.sub(
name, cfg['meta']['URL'])

if self._is_validate():
if self._is_validate:
self.mem_log("config.py: before _check_circular()")
self._check_circular()
self.mem_log("config.py: after _check_circular()")
Expand Down Expand Up @@ -980,12 +987,10 @@ def _expand_runtime(self):
expanded_node_attrs[name] = val
self.cfg['visualization']['node attributes'] = expanded_node_attrs

def _is_validate(self, is_strict=False):
"""Return whether we are in (strict) validate mode."""
return (
getattr(self.options, 'is_validate', False) and
(not is_strict or getattr(self.options, 'strict', False))
)
@property
def _is_validate(self) -> bool:
"""Return whether we are in validate mode."""
return getattr(self.options, 'is_validate', False)

@staticmethod
def dequote(s):
Expand Down Expand Up @@ -1496,7 +1501,8 @@ def _check_task_event_handlers(self):
)

def _check_special_tasks(self):
# Check declared special tasks are valid.
"""Check declared special tasks are valid, and detect special
implicit tasks"""
for task_type in self.cfg['scheduling']['special tasks']:
for name in self.cfg['scheduling']['special tasks'][task_type]:
if task_type in ['clock-trigger', 'clock-expire',
Expand All @@ -1507,7 +1513,7 @@ def _check_special_tasks(self):
f'Illegal {task_type} task name: {name}')
if (name not in self.taskdefs and
name not in self.cfg['runtime']):
self.naked_tasks.append(name)
self.implicit_tasks.add(name)

def _check_explicit_cycling(self):
"""Check that inter-cycle offsets refer to cycling tasks.
Expand Down Expand Up @@ -1569,8 +1575,8 @@ def generate_taskdefs(self, orig_expr, left_nodes, right, seq, suicide):
GraphNodeParser.get_inst().parse(node))

if name not in self.cfg['runtime']:
# naked task, implicit inheritance from root
self.naked_tasks.append(name)
# implicit inheritance from root
self.implicit_tasks.add(name)
# These can't just be a reference to root runtime as we have to
# make some items task-specific: e.g. subst task name in URLs.
self.cfg['runtime'][name] = OrderedDictWithDefaults()
Expand Down Expand Up @@ -1751,7 +1757,7 @@ def get_graph_raw(self, start_point_string, stop_point_string,
In validate mode, set ungroup_all to True, and only return non-suicide
edges with left and right nodes.
"""
is_validate = self._is_validate() # this is for _check_circular
is_validate = self._is_validate # this is for _check_circular
if is_validate:
ungroup_all = True
if group_nodes is None:
Expand Down
29 changes: 10 additions & 19 deletions cylc/flow/scripts/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,6 @@
def get_option_parser():
parser = COP(__doc__, jset=True, prep=True, icp=True)

parser.add_option(
"--strict",
help="Fail any use of unsafe or experimental features. "
"Currently this just means naked dummy tasks (tasks with no "
"corresponding runtime section) as these may result from "
"unintentional typographic errors in task names.",
action="store_true", default=False, dest="strict")

parser.add_option(
"--check-circular",
help="Check for circular dependencies in graphs when the number of "
Expand Down Expand Up @@ -107,17 +99,16 @@ def main(_, options, reg):
if out_of_bounds:
if len(out_of_bounds) > 1:
# avoid spamming users with multiple warnings
msg = ('multiple sequences out of bounds for initial cycle point '
'%s:\n%s' % (
cfg.start_point,
'\n'.join(textwrap.wrap(', '.join(out_of_bounds), 70))))
out_of_bounds_str = '\n'.join(
textwrap.wrap(', '.join(out_of_bounds), 70))
msg = (
"multiple sequences out of bounds for initial cycle point "
f"{cfg.start_point}:\n{out_of_bounds_str}")
else:
msg = '%s: sequence out of bounds for initial cycle point %s' % (
out_of_bounds[0], cfg.start_point)
if options.strict:
LOG.warning(msg)
elif cylc.flow.flags.verbose:
sys.stderr.write(' + %s\n' % msg)
msg = (
f"{out_of_bounds[0]}: sequence out of bounds for "
f"initial cycle point {cfg.start_point}")
LOG.warning(msg)

# Instantiate tasks and force evaluation of trigger expressions.
# (Taken from config.py to avoid circular import problems.)
Expand All @@ -129,7 +120,7 @@ def main(_, options, reg):
try:
itask = TaskProxy(taskdef, cfg.start_point, flow_label)
except TaskProxySequenceBoundsError:
# Should already failed above in strict mode.
# Should already failed above
mesg = 'Task out of bounds for %s: %s\n' % (cfg.start_point, name)
if cylc.flow.flags.verbose:
sys.stderr.write(' + %s\n' % mesg)
Expand Down
2 changes: 1 addition & 1 deletion tests/flakyfunctional/cyclers/19-async_integer/flow.cylc
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
[[graph]]
R1 = "foo => bar"
[runtime]
[[root]]
[[foo, bar]]
script = true
1 change: 1 addition & 0 deletions tests/flakyfunctional/cyclers/30-r1_at_icp_or/flow.cylc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[scheduler]
UTC mode = True
allow implicit tasks = True
[scheduling]
initial cycle point = 20130808T00
final cycle point = 20130809T18
Expand Down
1 change: 1 addition & 0 deletions tests/flakyfunctional/events/01-task/flow.cylc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
title = "test all event handlers"

[scheduler]
allow implicit tasks = True
[[events]]
abort on stalled = True
abort on inactivity = True
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ BASE_GLOBAL_CONFIG="
init_suite "${TEST_NAME_BASE}" <<< '
[scheduler]
UTC mode = True
allow implicit tasks = True
[scheduling]
initial cycle point = 2000
[[graph]]
Expand Down
2 changes: 2 additions & 0 deletions tests/flakyfunctional/restart/40-auto-restart-force-stop.t
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ BASE_GLOBAL_CONFIG="
#-------------------------------------------------------------------------------
# test the force shutdown option (auto stop, no restart) in condemned hosts
init_suite "${TEST_NAME_BASE}" <<< '
[scheduler]
allow implicit tasks = True
[scheduling]
[[graph]]
R1 = foo
Expand Down
2 changes: 2 additions & 0 deletions tests/flakyfunctional/shutdown/00-rm-db.t
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
set_test_number 5

init_suite "${TEST_NAME_BASE}" <<'__FLOW_CONFIG__'
[scheduler]
allow implicit tasks = True
[scheduling]
[[dependencies]]
R1 = foo
Expand Down
1 change: 1 addition & 0 deletions tests/functional/broadcast/00-simple/flow.cylc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ generated reference version.

[scheduler]
cycle point format = %Y%m%dT%H
allow implicit tasks = True

[scheduling]
initial cycle point = 20100808T00
Expand Down
1 change: 1 addition & 0 deletions tests/functional/clock-expire/00-basic/flow.cylc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ Skip a daily post-processing workflow if the 'copy' task has expired."""

[scheduler]
cycle point format = %Y-%m-%dT%H
allow implicit tasks = True
[[events]]
abort on timeout = True
timeout = PT1M
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/cyclers/0000_rollunder/flow.cylc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
[[graph]]
P1D = "foo[-P1Y] => foo"
[runtime]
[[root]]
[[foo]]
script = true
[visualization]
initial cycle point = 00000101T00
Expand Down
10 changes: 0 additions & 10 deletions tests/functional/cyclers/28-implicit-disallowed/flow.cylc

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env bash
# THIS FILE IS PART OF THE CYLC SUITE ENGINE.
# Copyright (C) NIWA & British Crown (Met Office) & Contributors.
#
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
Expand All @@ -15,12 +15,19 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#-------------------------------------------------------------------------------
# Check that missing explicit cycling fails validation.
# Check that missing cycling task fails validation.
. "$(dirname "$0")/test_header"
#-------------------------------------------------------------------------------
set_test_number 2
#-------------------------------------------------------------------------------
install_suite "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"
init_suite "${TEST_NAME_BASE}" << __FLOW__
[scheduling]
initial cycle point = 20140808T00
[[graph]]
P1D = foo[-P1D] => bar
[runtime]
[[foo,bar]]
__FLOW__
#-------------------------------------------------------------------------------
TEST_NAME="${TEST_NAME_BASE}-validate"
run_fail "${TEST_NAME}" cylc validate "${SUITE_NAME}"
Expand Down
1 change: 1 addition & 0 deletions tests/functional/cyclers/36-icp_fcp_notation/flow.cylc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[scheduler]
UTC mode = true
allow implicit tasks = True
[scheduling]
initial cycle point = 20160101T00Z
final cycle point = 20160102T00Z
Expand Down
1 change: 1 addition & 0 deletions tests/functional/cyclers/47-icp_fcp_notation/flow.cylc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[scheduler]
UTC mode = true
allow implicit tasks = True
[scheduling]
initial cycle point = 20160101T00Z
final cycle point = 20160102T00Z
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/cyclers/9999_rollover/flow.cylc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
[[graph]]
R3//PT1H = "foo"
[runtime]
[[root]]
[[foo]]
script = true
[visualization]
initial cycle point = 99991231T2200
Expand Down
1 change: 1 addition & 0 deletions tests/functional/cyclers/aeon/flow.cylc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
[scheduler]
cycle point num expanded year digits = 7
UTC mode = True
allow implicit tasks = True
[scheduling]
initial cycle point = -13800000000-02-29T05:30 # Big Bang
final cycle point = +05400000000-12-31T23:59 # Sun leaves main sequence
Expand Down
1 change: 1 addition & 0 deletions tests/functional/cyclers/daily/flow.cylc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[scheduler]
UTC mode = True
allow implicit tasks = True
[scheduling]
initial cycle point = 20131231T2300
final cycle point = 20140103T0000
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/cyclers/daily_final/flow.cylc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
[[graph]]
R//T00 = "daily_foo"
[runtime]
[[root]]
[[daily_foo]]
script = true
[visualization]
initial cycle point = 20140101
Expand Down
Loading