From f4fc43634e9558048c26ce8efc8d0758a80eee56 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Wed, 25 Jan 2023 11:50:26 +0000 Subject: [PATCH] treat items in queues as implicit tasks; queues to be list separated type (#5312) * treat items in queues as implicit tasks * made tasks implied by queues a warning only. Co-authored-by: Oliver Sanders --- CHANGES.md | 3 ++- cylc/flow/cfgspec/workflow.py | 2 +- cylc/flow/config.py | 36 ++++++++++++++++++++++++++++ tests/integration/test_config.py | 40 ++++++++++++++++++++++++++++++++ tests/unit/test_config.py | 22 ++++++++++++++++++ 5 files changed, 101 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 78c5a754ad7..a63a41e7819 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -15,10 +15,11 @@ ones in. --> ### Fixes +[#5312](https://github.com/cylc/cylc-flow/pull/5312) - task names must be comma-separated in queue member lists. Any implicit tasks (i.e. with no task definition under runtime) assigned to a queue will generate a warning. + [#5314](https://github.com/cylc/cylc-flow/pull/5314) - Fix broken command option: `cylc vip --run-name`. - ## __cylc-8.1.0 (Released 2023-01-16)__ ### Breaking Changes diff --git a/cylc/flow/cfgspec/workflow.py b/cylc/flow/cfgspec/workflow.py index b982c971866..dc7e4feb625 100644 --- a/cylc/flow/cfgspec/workflow.py +++ b/cylc/flow/cfgspec/workflow.py @@ -688,7 +688,7 @@ def get_script_common_text(this: str, example: Optional[str] = None): If set to 0 this queue is not limited. ''') - Conf('members', VDR.V_STRING_LIST, desc=''' + Conf('members', VDR.V_SPACELESS_STRING_LIST, desc=''' A list of member tasks, or task family names to assign to this queue. diff --git a/cylc/flow/config.py b/cylc/flow/config.py index 995af32d267..a549c9ed771 100644 --- a/cylc/flow/config.py +++ b/cylc/flow/config.py @@ -224,6 +224,7 @@ class WorkflowConfig: CHECK_CIRCULAR_LIMIT = 100 # If no. tasks > this, don't check circular VIS_N_POINTS = 3 + MAX_WARNING_LINES = 5 def __init__( self, @@ -532,6 +533,9 @@ def __init__( self._check_special_tasks() # adds to self.implicit_tasks self._check_explicit_cycling() + self._warn_if_queues_have_implicit_tasks( + self.cfg, self.taskdefs, self.MAX_WARNING_LINES) + self._check_implicit_tasks() self._check_sequence_bounds() self.validate_namespace_names() @@ -571,6 +575,38 @@ def __init__( self.mem_log("config.py: end init config") + @staticmethod + def _warn_if_queues_have_implicit_tasks( + config, taskdefs, max_warning_lines + ): + """Warn if queues contain implict tasks. + """ + implicit_q_msg = '' + + # Get the names of the first N implicit queue tasks: + for queue in config["scheduling"]["queues"]: + for name in config["scheduling"]["queues"][queue][ + "members" + ]: + if ( + name not in taskdefs + and name not in config['runtime'] + and len(implicit_q_msg.split('\n')) <= max_warning_lines + ): + implicit_q_msg += f'\n * task {name!r} in queue {queue!r}' + + # Warn users if tasks are implied by queues. + if implicit_q_msg: + truncation_msg = ( + f"\n...showing first {max_warning_lines} tasks..." + if len(implicit_q_msg.split('\n')) > max_warning_lines + else "" + ) + LOG.warning( + 'Queues contain tasks not defined in' + f' runtime: {implicit_q_msg}{truncation_msg}' + ) + def prelim_process_graph(self) -> None: """Ensure graph is not empty; set integer cycling mode and icp/fcp = 1 for simplest "R1 = foo" type graphs. diff --git a/tests/integration/test_config.py b/tests/integration/test_config.py index a73449025be..fc9d1d38ed3 100644 --- a/tests/integration/test_config.py +++ b/tests/integration/test_config.py @@ -17,6 +17,7 @@ import pytest from cylc.flow.exceptions import WorkflowConfigError +from cylc.flow.parsec.exceptions import ListValueError @pytest.mark.parametrize( @@ -282,3 +283,42 @@ def test_parse_special_tasks_families(flow, scheduler, validate, section): 'foo(P1D)', 'foot(P1D)' } + + +def test_queue_treated_as_implicit(flow, validate, caplog): + """Tasks in queues but not in runtime generate a warning. + + https://github.com/cylc/cylc-flow/issues/5260 + """ + reg = flow( + { + "scheduling": { + "queues": {"my_queue": {"members": "task1, task2"}}, + "graph": {"R1": "task2"}, + }, + "runtime": {"task2": {}}, + } + ) + validate(reg) + assert ( + 'Queues contain tasks not defined in runtime' + in caplog.records[0].message + ) + + +def test_queue_treated_as_comma_separated(flow, validate): + """Tasks listed in queue should be separated with commas, not spaces. + + https://github.com/cylc/cylc-flow/issues/5260 + """ + reg = flow( + { + "scheduling": { + "queues": {"my_queue": {"members": "task1 task2"}}, + "graph": {"R1": "task2"}, + }, + "runtime": {"task1": {}, "task2": {}}, + } + ) + with pytest.raises(ListValueError, match="cannot contain a space"): + validate(reg) diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 86e029a877b..5d43398730d 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -1543,3 +1543,25 @@ def test_find_taskdefs( assert sorted( t.name for t in awe_config.find_taskdefs(name) ) == sorted(expected) + + +def test__warn_if_queues_have_implicit_tasks(caplog): + """It Warns that queues imply tasks undefined in runtime. + """ + config = { + 'scheduling': {'queues': { + 'q1': {'members': ['foo']}, + 'q2': {'members': ['bar', 'baz']} + }}, + 'runtime': {} + } + taskdefs = {} + max_warning_lines = 2 + WorkflowConfig._warn_if_queues_have_implicit_tasks( + config, taskdefs, max_warning_lines) + result = caplog.records[0].message + assert "'foo' in queue 'q1'" in result + assert "'bar' in queue 'q2'" in result + assert "'baz'" not in result + assert f"showing first {max_warning_lines}" in result +