Clean up expire validation.

cylc · Jan 22, 2024 · 515a521 · 515a521
1 parent 5839f29
commit 515a521
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 16 deletions.
diff --git a/cylc/flow/config.py b/cylc/flow/config.py
@@ -2142,28 +2142,26 @@ def set_required_outputs(
                     continue
                 taskdef.set_required_output(output, not optional)
 
-        # Expire is dangerous, it must be visible and optional in the graph
-        bad_exp = set()
-        good_exp = set()
-        for (task, output), (opt, _, _) in task_output_opt.items():
+        # Add expired outputs to taskdefs if flagged in the graph.
+        graph_exp = set()
+        for (task, output) in task_output_opt.keys():
             if output == TASK_OUTPUT_EXPIRED:
-                if not opt:
-                    bad_exp.add(task)
-                    continue
-                good_exp.add(task)
+                graph_exp.add(task)
                 self.taskdefs[task].add_output(
                     TASK_OUTPUT_EXPIRED, TASK_OUTPUT_EXPIRED
                 )
 
-        # likewise clock-expiry is only legal if flagged in the graph
+        # clock-expire must be flagged in the graph for visibility
+        bad_exp = set()
         for task in self.expiration_offsets:
-            if task not in good_exp:
+            if task not in graph_exp:
                 bad_exp.add(task)
 
         if bad_exp:
-            msg = '\n   '.join([t + ":expired?" for t in bad_exp])
+            msg = '\n   '.join(
+                [t + f":{TASK_OUTPUT_EXPIRED}?" for t in bad_exp])
             raise WorkflowConfigError(
-                f"Expiring tasks must be optional in the graph as:\n   {msg}"
+                f"Clock-expire must be visible in the graph:\n  {msg}"
             )
 
     def find_taskdefs(self, name: str) -> Set[TaskDef]:

diff --git a/cylc/flow/graph_parser.py b/cylc/flow/graph_parser.py
@@ -61,6 +61,7 @@
 
 class Replacement:
     """A class to remember match group information in re.sub() calls"""
+
     def __init__(self, replacement):
         self.replacement = replacement
         self.substitutions = []
@@ -745,6 +746,10 @@ def _set_output_opt(
         if suicide:
             return
 
+        if output == TASK_OUTPUT_EXPIRED and not optional:
+            raise GraphParseError(
+                f"Output {name}:{output} must be optional (append '?')")
+
         if output == TASK_OUTPUT_FINISHED:
             # Interpret :finish pseudo-output
             if optional:

diff --git a/cylc/flow/task_job_mgr.py b/cylc/flow/task_job_mgr.py
@@ -183,9 +183,9 @@ def kill_task_jobs(self, itasks, expire=False):
         if expire: expire tasks (in the callback) after killing them.
 
         """
+        ok = True
         if expire:
             # Check these tasks are allowed to expire.
-            ok = True
             output = TASK_OUTPUT_EXPIRED
             for itask in itasks:
                 msg = itask.state.outputs.get_msg(output)

diff --git a/tests/integration/scripts/test_completion_server.py b/tests/integration/scripts/test_completion_server.py
@@ -91,7 +91,6 @@ async def test_list_prereqs_and_outputs(flow, scheduler, start):
         # list outputs (b1)
         assert await _complete_cylc('cylc', 'set', b1.id, '--out', '') == {
             # regular task outputs
-            'expired',
             'failed',
             'started',
             'submit-failed',

diff --git a/tests/integration/test_config.py b/tests/integration/test_config.py
@@ -263,6 +263,13 @@ def test_parse_special_tasks_families(flow, scheduler, validate, section):
         with pytest.raises(WorkflowConfigError) as exc_ctx:
             config = validate(id_)
         assert 'external triggers must be used only once' in str(exc_ctx.value)
+
+    elif section == 'clock-expire':
+        with pytest.raises(WorkflowConfigError) as exc_ctx:
+            config = validate(id_)
+        assert (
+            'Clock-expire must be visible in the graph' in str(exc_ctx.value)
+        )
     else:
         config = validate(id_)
         assert set(config.cfg['scheduling']['special tasks'][section]) == {

diff --git a/tests/integration/test_task_job_mgr.py b/tests/integration/test_task_job_mgr.py
@@ -99,7 +99,6 @@ async def test_run_job_cmd_no_hosts_error(
 
         # killing the task should not result in an error...
         schd.task_job_mgr.kill_task_jobs(
-            schd.workflow,
             schd.pool.get_tasks()
         )
 

diff --git a/tests/integration/tui/screenshots/test_show.success.html b/tests/integration/tui/screenshots/test_show.success.html
@@ -17,7 +17,6 @@
 <span style="color:#000000;background:#e5e5e5">               </span><span style="color:#000000;background:#e5e5e5">│</span><span style="color:#000000;background:#e5e5e5">  </span><span style="color:#000000;background:#e5e5e5">state: waiting                              </span><span style="color:#000000;background:#e5e5e5">  </span><span style="color:#000000;background:#e5e5e5">│</span><span style="color:#000000;background:#e5e5e5">               </span>
 <span style="color:#000000;background:#e5e5e5">               </span><span style="color:#000000;background:#e5e5e5">│</span><span style="color:#000000;background:#e5e5e5">  </span><span style="color:#000000;background:#e5e5e5">prerequisites: (None)                       </span><span style="color:#000000;background:#e5e5e5">  </span><span style="color:#000000;background:#e5e5e5">│</span><span style="color:#000000;background:#e5e5e5">               </span>
 <span style="color:#000000;background:#e5e5e5">               </span><span style="color:#000000;background:#e5e5e5">│</span><span style="color:#000000;background:#e5e5e5">  </span><span style="color:#000000;background:#e5e5e5">outputs: ('-': not completed)               </span><span style="color:#000000;background:#e5e5e5">  </span><span style="color:#000000;background:#e5e5e5">│</span><span style="color:#000000;background:#e5e5e5">               </span>
-<span style="color:#000000;background:#e5e5e5">               </span><span style="color:#000000;background:#e5e5e5">│</span><span style="color:#000000;background:#e5e5e5">  </span><span style="color:#000000;background:#e5e5e5">  - 1/foo expired                           </span><span style="color:#000000;background:#e5e5e5">  </span><span style="color:#000000;background:#e5e5e5">│</span><span style="color:#000000;background:#e5e5e5">               </span>
 <span style="color:#000000;background:#e5e5e5">               </span><span style="color:#000000;background:#e5e5e5">│</span><span style="color:#000000;background:#e5e5e5">  </span><span style="color:#000000;background:#e5e5e5">  - 1/foo submitted                         </span><span style="color:#000000;background:#e5e5e5">  </span><span style="color:#000000;background:#e5e5e5">│</span><span style="color:#000000;background:#e5e5e5">               </span>
 <span style="color:#000000;background:#e5e5e5">               </span><span style="color:#000000;background:#e5e5e5">│</span><span style="color:#000000;background:#e5e5e5">  </span><span style="color:#000000;background:#e5e5e5">  - 1/foo submit-failed                     </span><span style="color:#000000;background:#e5e5e5">  </span><span style="color:#000000;background:#e5e5e5">│</span><span style="color:#000000;background:#e5e5e5">               </span>
 <span style="color:#000000;background:#e5e5e5">               </span><span style="color:#000000;background:#e5e5e5">│</span><span style="color:#000000;background:#e5e5e5">  </span><span style="color:#000000;background:#e5e5e5">  - 1/foo started                           </span><span style="color:#000000;background:#e5e5e5">  </span><span style="color:#000000;background:#e5e5e5">│</span><span style="color:#000000;background:#e5e5e5">               </span>
@@ -36,6 +35,7 @@
 <span style="color:#000000;background:#e5e5e5">                                                                                </span>
 <span style="color:#000000;background:#e5e5e5">                                                                                </span>
 <span style="color:#000000;background:#e5e5e5">                                                                                </span>
+<span style="color:#000000;background:#e5e5e5">                                                                                </span>
 <span style="color:#ffffff;background:#0000ee">quit: </span><span style="color:#00ffff;background:#0000ee">q</span><span style="color:#ffffff;background:#0000ee">  help: </span><span style="color:#00ffff;background:#0000ee">h</span><span style="color:#ffffff;background:#0000ee">  context: </span><span style="color:#00ffff;background:#0000ee">enter</span><span style="color:#ffffff;background:#0000ee">  tree: </span><span style="color:#00ffff;background:#0000ee">- ← + → </span><span style="color:#ffffff;background:#0000ee"> navigation: </span><span style="color:#00ffff;background:#0000ee">↑ ↓ ↥ ↧ Home End </span><span style="color:#ffffff;background:#0000ee">  </span>
 <span style="color:#ffffff;background:#0000ee">filter tasks: </span><span style="color:#00ffff;background:#0000ee">T f s r R </span><span style="color:#ffffff;background:#0000ee"> filter workflows: </span><span style="color:#00ffff;background:#0000ee">W E p </span><span style="color:#ffffff;background:#0000ee">                               </span>
 </pre>