From 73a597e3759ca553fe7a3af03a66daad4fd1b4c1 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 10 Jun 2024 20:52:00 +0100 Subject: [PATCH 1/9] Added validation for tasks to prevent adding jobs to tasks where that did not work --- changes.d/6130.fix.md | 1 + cylc/flow/command_validation.py | 37 +++++++++++++++++++++++++++++++++ cylc/flow/commands.py | 6 ++++++ 3 files changed, 44 insertions(+) create mode 100644 changes.d/6130.fix.md diff --git a/changes.d/6130.fix.md b/changes.d/6130.fix.md new file mode 100644 index 00000000000..0db1bab7002 --- /dev/null +++ b/changes.d/6130.fix.md @@ -0,0 +1 @@ +Prevent commands taking a list of task ID's from submitting job ids. \ No newline at end of file diff --git a/cylc/flow/command_validation.py b/cylc/flow/command_validation.py index c7a9b2762dc..3f253fa33c3 100644 --- a/cylc/flow/command_validation.py +++ b/cylc/flow/command_validation.py @@ -228,3 +228,40 @@ def consistency( """ if outputs and prereqs: raise InputError("Use --prerequisite or --output, not both.") + + +def is_tasks(tasks: List[str]): + """All tasks in a list of tasks are task ID's + without trailing job ID. + + Examples: + + # All legal + >>> is_tasks(['1/foo', '1/bar', '*/baz', '*/*']) + True + + # Some legal + >>> is_tasks(['1/foo/NN', '1/bar', '*/baz', '*/*/42']) + Traceback (most recent call last): + ... + Exception: This command does not take job ids: + * 1/foo/NN + * */*/42 + + # None legal + >>> is_tasks(['1/', '*/baz/12']) + Traceback (most recent call last): + ... + Exception: This command does not take job ids: + * 1/ + * */baz/12 + """ + bad_tasks: List[str] = [] + for task in tasks: + tokens = Tokens('//' + task) + if tokens.lowest_token < 'task': + bad_tasks.append(task) + if bad_tasks: + msg = 'This command does not take job ids:\n * ' + raise Exception(msg + '\n * '.join(bad_tasks)) + return True diff --git a/cylc/flow/commands.py b/cylc/flow/commands.py index 28de0d16ed5..77a8ed8b97c 100644 --- a/cylc/flow/commands.py +++ b/cylc/flow/commands.py @@ -214,6 +214,7 @@ async def stop( @_command('release') async def release(schd: 'Scheduler', tasks: Iterable[str]): """Release held tasks.""" + validate.is_tasks(tasks) yield yield schd.pool.release_held_tasks(tasks) @@ -237,6 +238,7 @@ async def resume(schd: 'Scheduler'): @_command('poll_tasks') async def poll_tasks(schd: 'Scheduler', tasks: Iterable[str]): """Poll pollable tasks or a task or family if options are provided.""" + validate.is_tasks(tasks) yield if schd.get_run_mode() == RunMode.SIMULATION: yield 0 @@ -248,6 +250,7 @@ async def poll_tasks(schd: 'Scheduler', tasks: Iterable[str]): @_command('kill_tasks') async def kill_tasks(schd: 'Scheduler', tasks: Iterable[str]): """Kill all tasks or a task/family if options are provided.""" + validate.is_tasks(tasks) yield itasks, _, bad_items = schd.pool.filter_task_proxies(tasks) if schd.get_run_mode() == RunMode.SIMULATION: @@ -264,6 +267,7 @@ async def kill_tasks(schd: 'Scheduler', tasks: Iterable[str]): @_command('hold') async def hold(schd: 'Scheduler', tasks: Iterable[str]): """Hold specified tasks.""" + validate.is_tasks(tasks) yield yield schd.pool.hold_tasks(tasks) @@ -304,6 +308,7 @@ async def set_verbosity(schd: 'Scheduler', level: Union[int, str]): @_command('remove_tasks') async def remove_tasks(schd: 'Scheduler', tasks: Iterable[str]): """Remove tasks.""" + validate.is_tasks(tasks) yield yield schd.pool.remove_tasks(tasks) @@ -430,5 +435,6 @@ async def force_trigger_tasks( flow_descr: Optional[str] = None, ): """Manual task trigger.""" + validate.is_tasks(tasks) yield yield schd.pool.force_trigger_tasks(tasks, flow, flow_wait, flow_descr) From c7e9468b61f1132fa48713c499b64474abd66bb9 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Wed, 19 Jun 2024 11:07:12 +0100 Subject: [PATCH 2/9] Update cylc/flow/command_validation.py --- cylc/flow/command_validation.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cylc/flow/command_validation.py b/cylc/flow/command_validation.py index 3f253fa33c3..dfea7036045 100644 --- a/cylc/flow/command_validation.py +++ b/cylc/flow/command_validation.py @@ -233,7 +233,6 @@ def consistency( def is_tasks(tasks: List[str]): """All tasks in a list of tasks are task ID's without trailing job ID. - Examples: # All legal From 918086bd2c9210ddad4b180570fac7dfc79e26d8 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Wed, 19 Jun 2024 11:07:43 +0100 Subject: [PATCH 3/9] Update cylc/flow/command_validation.py --- cylc/flow/command_validation.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cylc/flow/command_validation.py b/cylc/flow/command_validation.py index dfea7036045..8b5e3ec3731 100644 --- a/cylc/flow/command_validation.py +++ b/cylc/flow/command_validation.py @@ -233,6 +233,7 @@ def consistency( def is_tasks(tasks: List[str]): """All tasks in a list of tasks are task ID's without trailing job ID. + Examples: # All legal From d9dd0c30f77414a59fb81229310e23b50b007f36 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Wed, 19 Jun 2024 11:49:49 +0100 Subject: [PATCH 4/9] minor correction --- cylc/flow/command_validation.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cylc/flow/command_validation.py b/cylc/flow/command_validation.py index 8b5e3ec3731..50765496169 100644 --- a/cylc/flow/command_validation.py +++ b/cylc/flow/command_validation.py @@ -253,13 +253,12 @@ def is_tasks(tasks: List[str]): Traceback (most recent call last): ... Exception: This command does not take job ids: - * 1/ * */baz/12 """ bad_tasks: List[str] = [] for task in tasks: tokens = Tokens('//' + task) - if tokens.lowest_token < 'task': + if tokens.lowest_token == 'job': bad_tasks.append(task) if bad_tasks: msg = 'This command does not take job ids:\n * ' From 1bd4cef4edc922ccb09ec3f51b3268436adcbdff Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Wed, 19 Jun 2024 11:53:48 +0100 Subject: [PATCH 5/9] Bugbear fix --- cylc/flow/command_validation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cylc/flow/command_validation.py b/cylc/flow/command_validation.py index 50765496169..b20dcee8d3c 100644 --- a/cylc/flow/command_validation.py +++ b/cylc/flow/command_validation.py @@ -23,7 +23,7 @@ ) from cylc.flow.exceptions import InputError -from cylc.flow.id import Tokens +from cylc.flow.id import IDTokens, Tokens from cylc.flow.task_outputs import TASK_OUTPUT_SUCCEEDED from cylc.flow.flow_mgr import FLOW_ALL, FLOW_NEW, FLOW_NONE @@ -258,7 +258,7 @@ def is_tasks(tasks: List[str]): bad_tasks: List[str] = [] for task in tasks: tokens = Tokens('//' + task) - if tokens.lowest_token == 'job': + if tokens.lowest_token == IDTokens.Job: bad_tasks.append(task) if bad_tasks: msg = 'This command does not take job ids:\n * ' From 049b9b3d5d212dae9a58d7815387b29a1adebe1e Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Wed, 19 Jun 2024 12:50:13 +0100 Subject: [PATCH 6/9] fix test --- cylc/flow/command_validation.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cylc/flow/command_validation.py b/cylc/flow/command_validation.py index b20dcee8d3c..c22e54b3231 100644 --- a/cylc/flow/command_validation.py +++ b/cylc/flow/command_validation.py @@ -18,6 +18,7 @@ from typing import ( + Iterable, List, Optional, ) @@ -230,7 +231,7 @@ def consistency( raise InputError("Use --prerequisite or --output, not both.") -def is_tasks(tasks: List[str]): +def is_tasks(tasks: Iterable[str]): """All tasks in a list of tasks are task ID's without trailing job ID. @@ -249,7 +250,7 @@ def is_tasks(tasks: List[str]): * */*/42 # None legal - >>> is_tasks(['1/', '*/baz/12']) + >>> is_tasks(['*/baz/12']) Traceback (most recent call last): ... Exception: This command does not take job ids: @@ -258,7 +259,7 @@ def is_tasks(tasks: List[str]): bad_tasks: List[str] = [] for task in tasks: tokens = Tokens('//' + task) - if tokens.lowest_token == IDTokens.Job: + if tokens.lowest_token == IDTokens.Job.value: bad_tasks.append(task) if bad_tasks: msg = 'This command does not take job ids:\n * ' From 7d800ab89a30fc128737ecd1ffdacad30911e032 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 20 Jun 2024 12:53:05 +0100 Subject: [PATCH 7/9] re-implement suggestions from review and fix test --- cylc/flow/command_validation.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/cylc/flow/command_validation.py b/cylc/flow/command_validation.py index c22e54b3231..1c57d452c43 100644 --- a/cylc/flow/command_validation.py +++ b/cylc/flow/command_validation.py @@ -232,20 +232,17 @@ def consistency( def is_tasks(tasks: Iterable[str]): - """All tasks in a list of tasks are task ID's - without trailing job ID. + """All tasks in a list of tasks are task ID's without trailing job ID. Examples: - # All legal >>> is_tasks(['1/foo', '1/bar', '*/baz', '*/*']) - True # Some legal >>> is_tasks(['1/foo/NN', '1/bar', '*/baz', '*/*/42']) Traceback (most recent call last): ... - Exception: This command does not take job ids: + cylc.flow.exceptions.InputError: This command does not take job ids: * 1/foo/NN * */*/42 @@ -253,7 +250,7 @@ def is_tasks(tasks: Iterable[str]): >>> is_tasks(['*/baz/12']) Traceback (most recent call last): ... - Exception: This command does not take job ids: + cylc.flow.exceptions.InputError: This command does not take job ids: * */baz/12 """ bad_tasks: List[str] = [] @@ -263,5 +260,4 @@ def is_tasks(tasks: Iterable[str]): bad_tasks.append(task) if bad_tasks: msg = 'This command does not take job ids:\n * ' - raise Exception(msg + '\n * '.join(bad_tasks)) - return True + raise InputError(msg + '\n * '.join(bad_tasks)) From 9e4eaa730d10019147e2ffb44e45b5fa9c15a518 Mon Sep 17 00:00:00 2001 From: WXTIM <26465611+wxtim@users.noreply.github.com> Date: Fri, 21 Jun 2024 10:06:27 +0100 Subject: [PATCH 8/9] check for valid tasks for set and stop --- cylc/flow/commands.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cylc/flow/commands.py b/cylc/flow/commands.py index 77a8ed8b97c..a4ea43df5cf 100644 --- a/cylc/flow/commands.py +++ b/cylc/flow/commands.py @@ -145,6 +145,7 @@ async def set_prereqs_and_outputs( outputs = validate.outputs(outputs) prerequisites = validate.prereqs(prerequisites) validate.flow_opts(flow, flow_wait) + validate.is_tasks(tasks) yield @@ -172,6 +173,8 @@ async def stop( task: Optional[str] = None, flow_num: Optional[int] = None, ): + if task: + validate.is_tasks([task]) yield if flow_num: schd.pool.stop_flow(flow_num) From 65c5102f509ca3809372f232cf182c85cbeea4c0 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 24 Jun 2024 12:57:00 +0100 Subject: [PATCH 9/9] Update changes.d/6130.fix.md Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- changes.d/6130.fix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes.d/6130.fix.md b/changes.d/6130.fix.md index 0db1bab7002..8423bfdcd40 100644 --- a/changes.d/6130.fix.md +++ b/changes.d/6130.fix.md @@ -1 +1 @@ -Prevent commands taking a list of task ID's from submitting job ids. \ No newline at end of file +Prevent commands accepting job IDs where it doesn't make sense. \ No newline at end of file