treat items in queues as implicit tasks; queues to be list separated …

…type (cylc#5312) * treat items in queues as implicit tasks * made tasks implied by queues a warning only. Co-authored-by: Oliver Sanders <[email protected]>
wxtim · Jan 25, 2023 · f4fc436 · f4fc436
1 parent eb60339
commit f4fc436
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 2 deletions.
diff --git 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 (<span actions:bind='release-date'>Released 2023-01-16</span>)__
 
 ### Breaking Changes

diff --git 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
@@ -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
@@ -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
@@ -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
+