From eec08d38840e12c7e8b1771836d75433fbcda0f0 Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Mon, 22 Jan 2024 11:33:30 +1300 Subject: [PATCH] Clean up expire validation. --- cylc/flow/config.py | 22 +++++++++---------- cylc/flow/graph_parser.py | 5 +++++ cylc/flow/task_job_mgr.py | 3 +-- .../scripts/test_completion_server.py | 1 - tests/integration/test_config.py | 7 ++++++ tests/integration/test_task_job_mgr.py | 1 - .../tui/screenshots/test_show.success.html | 2 +- 7 files changed, 24 insertions(+), 17 deletions(-) diff --git a/cylc/flow/config.py b/cylc/flow/config.py index b536f52c702..93a49a435c3 100644 --- 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 index ab3890961f9..090ebae1c5a 100644 --- 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 index 3b5b1bcd1a1..49590245e38 100644 --- a/cylc/flow/task_job_mgr.py +++ b/cylc/flow/task_job_mgr.py @@ -104,7 +104,6 @@ TASK_STATUS_SUBMITTED, TASK_STATUS_RUNNING, TASK_STATUS_WAITING, - TASK_STATUS_EXPIRED, TASK_STATUSES_ACTIVE ) from cylc.flow.wallclock import ( @@ -183,9 +182,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 index 24375c31f27..db0fb2a57d6 100644 --- 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 index acf24d17eaf..dd8da5edb24 100644 --- 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 index b085162a1da..e1338a7ae77 100644 --- 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 index afdcd1a73b4..fe016f285df 100644 --- a/tests/integration/tui/screenshots/test_show.success.html +++ b/tests/integration/tui/screenshots/test_show.success.html @@ -17,7 +17,6 @@ │ state: waiting │ │ prerequisites: (None) │ │ outputs: ('-': not completed) │ - │ - 1/foo expired │ │ - 1/foo submitted │ │ - 1/foo submit-failed │ │ - 1/foo started │ @@ -36,6 +35,7 @@ + quit: q help: h context: enter tree: - ← + → navigation: ↑ ↓ ↥ ↧ Home End filter tasks: T f s r R filter workflows: W E p \ No newline at end of file