From 995ab27a9e96154f1028e7facec77029b6c99b76 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Tue, 12 Nov 2024 11:15:48 +0000 Subject: [PATCH] Response to review --- cylc/flow/run_modes/simulation.py | 2 -- cylc/flow/scheduler.py | 4 ++-- cylc/flow/task_job_mgr.py | 11 +++-------- cylc/flow/task_pool.py | 6 +++--- cylc/flow/taskdef.py | 2 +- tests/unit/run_modes/test_skip_units.py | 17 +++++++++++------ tests/unit/test_task_state.py | 8 ++++---- tests/unit/test_xtrigger_mgr.py | 2 -- 8 files changed, 24 insertions(+), 28 deletions(-) diff --git a/cylc/flow/run_modes/simulation.py b/cylc/flow/run_modes/simulation.py index 47f234b0b2b..900a2c1fc4f 100644 --- a/cylc/flow/run_modes/simulation.py +++ b/cylc/flow/run_modes/simulation.py @@ -76,8 +76,6 @@ def submit_task_job( 'name': RunMode.SIMULATION.value, 'install target': 'localhost', 'hosts': ['localhost'], - 'disable task event handlers': - rtconfig['simulation']['disable task event handlers'], 'submission retry delays': [], 'execution retry delays': [] } diff --git a/cylc/flow/scheduler.py b/cylc/flow/scheduler.py index 7f924674c39..3632948fae3 100644 --- a/cylc/flow/scheduler.py +++ b/cylc/flow/scheduler.py @@ -114,7 +114,7 @@ from cylc.flow.workflow_db_mgr import WorkflowDatabaseManager from cylc.flow.workflow_events import WorkflowEventHandler from cylc.flow.workflow_status import StopMode, AutoRestartMode -from cylc.flow.run_modes import RunMode, WORKFLOW_ONLY_MODES +from cylc.flow.run_modes import RunMode from cylc.flow.taskdef import TaskDef from cylc.flow.task_events_mgr import TaskEventsManager from cylc.flow.task_job_mgr import TaskJobManager @@ -1264,7 +1264,7 @@ def run_event_handlers(self, event, reason=""): Run workflow events only in live mode or skip mode. """ - if self.get_run_mode().value in WORKFLOW_ONLY_MODES: + if self.get_run_mode() in {RunMode.SIMULATION, RunMode.DUMMY}: return self.workflow_event_handler.handle(self, event, str(reason)) diff --git a/cylc/flow/task_job_mgr.py b/cylc/flow/task_job_mgr.py index 1e5eff31161..ec8c84690dc 100644 --- a/cylc/flow/task_job_mgr.py +++ b/cylc/flow/task_job_mgr.py @@ -1074,14 +1074,9 @@ def submit_nonlive_task_jobs( # submission pipeline - We decide based on the output # of the submit method: submit_func = run_mode.get_submit_method() - if not submit_func: - # Return to nonlive. - nonlive_mode = False - else: - nonlive_mode = submit_func( - self, itask, rtconfig, workflow, now) - - if nonlive_mode: + if submit_func and submit_func( + self, itask, rtconfig, workflow, now + ): self.workflow_db_mgr.put_insert_task_states( itask, { diff --git a/cylc/flow/task_pool.py b/cylc/flow/task_pool.py index 0aab292b08a..8795e2814de 100644 --- a/cylc/flow/task_pool.py +++ b/cylc/flow/task_pool.py @@ -1993,12 +1993,12 @@ def _set_outputs_itask( # Check for broadcasts to task: bc_mgr = self.task_events_mgr.broadcast_mgr rtconfig = bc_mgr.get_updated_rtconfig(itask) - outputs.remove(RunMode.SKIP.value) skips = get_skip_mode_outputs(itask, rtconfig) itask.run_mode = RunMode.SKIP outputs = self._standardise_outputs( - itask.point, itask.tdef, outputs) - outputs = list(set(outputs + skips)) + itask.point, itask.tdef, outputs + ) + outputs = list(set(outputs + skips) - {RunMode.SKIP.value}) for output in sorted(outputs, key=itask.state.outputs.output_sort_key): if itask.state.outputs.is_message_complete(output): diff --git a/cylc/flow/taskdef.py b/cylc/flow/taskdef.py index 9dbecf1aa16..34461ca0d6f 100644 --- a/cylc/flow/taskdef.py +++ b/cylc/flow/taskdef.py @@ -409,7 +409,7 @@ def is_parentless(self, point): def __repr__(self) -> str: """ >>> TaskDef( - ... name='oliver', rtcfg={}, run_mode='fake', start_point='1', + ... name='oliver', rtcfg={}, start_point='1', ... initial_point='1' ... ) diff --git a/tests/unit/run_modes/test_skip_units.py b/tests/unit/run_modes/test_skip_units.py index 9c580117739..0b5316662ad 100644 --- a/tests/unit/run_modes/test_skip_units.py +++ b/tests/unit/run_modes/test_skip_units.py @@ -15,6 +15,7 @@ # along with this program. If not, see . """Unit tests for utilities supporting skip modes """ +import logging import pytest from pytest import param, raises from types import SimpleNamespace @@ -105,7 +106,7 @@ def test_process_outputs(outputs, required, expect): assert process_outputs(itask, rtconf) == ['submitted', 'started'] + expect -def test_skip_mode_validate(monkeypatch, caplog): +def test_skip_mode_validate(caplog, log_filter): """It warns us if we've set a task config to nonlive mode. (And not otherwise) @@ -128,8 +129,12 @@ def test_skip_mode_validate(monkeypatch, caplog): skip_mode_validate(taskdefs) - message = caplog.messages[0] - - assert 'skip mode:\n * skip_task' in message - assert ' live mode' not in message # Avoid matching "non-live mode" - assert 'workflow mode' not in message + assert len(caplog.records) == 1 + assert log_filter( + level=logging.WARNING, + exact_match=( + "The following tasks are set to run in skip mode:\n" + " * skip_task" + ), + log=caplog + ) diff --git a/tests/unit/test_task_state.py b/tests/unit/test_task_state.py index 7fc1feadcc6..7350a9aed74 100644 --- a/tests/unit/test_task_state.py +++ b/tests/unit/test_task_state.py @@ -41,7 +41,7 @@ ) def test_state_comparison(state, is_held): """Test the __call__ method.""" - tdef = TaskDef('foo', {}, 'live', '123', '123') + tdef = TaskDef('foo', {}, '123', '123') tstate = TaskState(tdef, '123', state, is_held) assert tstate(state, is_held=is_held) @@ -72,7 +72,7 @@ def test_state_comparison(state, is_held): ) def test_reset(state, is_held, should_reset): """Test that tasks do or don't have their state changed.""" - tdef = TaskDef('foo', {}, 'live', '123', '123') + tdef = TaskDef('foo', {}, '123', '123') # create task state: # * status: waiting # * is_held: true @@ -96,7 +96,7 @@ def test_task_prereq_duplicates(set_cycling_type): dep = Dependency([trig], [trig], False) - tdef = TaskDef('foo', {}, 'live', IntegerPoint("1"), IntegerPoint("1")) + tdef = TaskDef('foo', {}, IntegerPoint("1"), IntegerPoint("1")) tdef.add_dependency(dep, seq1) tdef.add_dependency(dep, seq2) # duplicate! @@ -110,7 +110,7 @@ def test_task_prereq_duplicates(set_cycling_type): def test_task_state_order(): """Test is_gt and is_gte methods.""" - tdef = TaskDef('foo', {}, 'live', IntegerPoint("1"), IntegerPoint("1")) + tdef = TaskDef('foo', {}, IntegerPoint("1"), IntegerPoint("1")) tstate = TaskState(tdef, IntegerPoint("1"), TASK_STATUS_SUBMITTED, False) assert tstate.is_gt(TASK_STATUS_WAITING) diff --git a/tests/unit/test_xtrigger_mgr.py b/tests/unit/test_xtrigger_mgr.py index 276fd354a95..2ec207cf25d 100644 --- a/tests/unit/test_xtrigger_mgr.py +++ b/tests/unit/test_xtrigger_mgr.py @@ -178,7 +178,6 @@ def test_housekeeping_with_xtrigger_satisfied(xtrigger_mgr): tdef = TaskDef( name="foo", rtcfg={'completion': None}, - run_mode="live", start_point=1, initial_point=1, ) @@ -232,7 +231,6 @@ def test__call_xtriggers_async(xtrigger_mgr): tdef = TaskDef( name="foo", rtcfg={'completion': None}, - run_mode="live", start_point=1, initial_point=1 )