Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Flow-specific hold/release #5698

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from
Draft

Flow-specific hold/release #5698

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift click to select a range
e8d957c
Task pool flow hold.
hjoliver Aug 16, 2023
58dc6c5
Future flow hold.
hjoliver Aug 17, 2023
95fa798
Flow-specific hold release.
hjoliver Aug 17, 2023
1e7d548
Centralise task hold/release code.
hjoliver Aug 20, 2023
b5f83a4
Style fixes.
hjoliver Aug 21, 2023
584bea8
Fix integration tests.
hjoliver Aug 22, 2023
77acf44
fix hold on kill
hjoliver Aug 22, 2023
ad81aa3
Fix func tests.
hjoliver Aug 22, 2023
2b570d5
Fix more func tests.
hjoliver Aug 24, 2023
ff07668
Update change log.
hjoliver Aug 28, 2023
d3c776c
Fix hold on kill.
hjoliver Aug 31, 2023
b2e7440
hold mgr: clear not needed.
hjoliver Aug 31, 2023
151537e
Validate CLI --flow opt.
hjoliver Aug 31, 2023
f554c96
Update flow hold integration tests.
hjoliver Sep 1, 2023
6597431
fix release from hold.
hjoliver Sep 1, 2023
e1fb7fe
Update flow release integration tests.
hjoliver Sep 1, 2023
9e93454
hold_point with flow num
hjoliver Sep 1, 2023
e912f63
Fix recent.
hjoliver Sep 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Validate CLI --flow opt.
  • Loading branch information
@hjoliver
hjoliver committed Aug 31, 2023
commit 151537e397478d63dba2463f8553c78e4f678a61
10 changes: 10 additions & 0 deletions cylc/flow/flow_mgr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 20,7 @@
import datetime

from cylc.flow import LOG
from cylc.flow.exceptions import InputError
from cylc.flow.workflow_db_mgr import WorkflowDatabaseManager


Expand All @@ -30,6 31,15 @@
FLOW_NONE = "none"


def validate_flow_opt(val):
Copy link
Member

@wxtim wxtim Oct 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this semi duplicate the logic in cylc trigger? Surely all use-cases of --flow should behave roughly the same, and probably be centralized in option_parser.py. Centralizing them here is reasonable too, but on balance this is more about options than flows?

You might want to give it a kwarg where you pass it a list of acceptable strings?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, there are some differences, e.g. cylc trigger --flow=new makes sense, but cylc hold --flow=new doesn't.

"""Validate command line --flow opions."""
if val is not None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will the parser allow this value to be None?

try:
int(val)
except ValueError:
raise InputError(f"--flow={val}: value must be integer.")


class FlowMgr:
"""Logic to manage flow counter and flow metadata."""

Expand Down
4 changes: 4 additions & 0 deletions cylc/flow/scripts/hold.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 59,7 @@
from typing import TYPE_CHECKING

from cylc.flow.exceptions import InputError
from cylc.flow.flow_mgr import validate_flow_opt
from cylc.flow.network.client_factory import get_client
from cylc.flow.option_parsers import (
FULL_ID_MULTI_ARG_DOC,
Expand All @@ -67,6 68,7 @@
from cylc.flow.terminal import cli_function
from cylc.flow.network.multi import call_multi


if TYPE_CHECKING:
from optparse import Values

Expand Down Expand Up @@ -136,6 138,8 @@ def _validate(options: 'Values', *task_globs: str) -> None:
raise InputError(
"Must define Cycles/Tasks. See `cylc hold --help`.")

validate_flow_opt(options.flow_num)


async def run(options, workflow_id, *tokens_list):
_validate(options, *tokens_list)
Expand Down
3 changes: 3 additions & 0 deletions cylc/flow/scripts/release.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 42,7 @@
from typing import TYPE_CHECKING

from cylc.flow.exceptions import InputError
from cylc.flow.flow_mgr import validate_flow_opt
from cylc.flow.network.client_factory import get_client
from cylc.flow.network.multi import call_multi
from cylc.flow.option_parsers import (
Expand Down Expand Up @@ -118,6 119,8 @@ def _validate(options: 'Values', *tokens_list: str) -> None:
"Must define Cycles/Tasks. See `cylc release --help`."
)

validate_flow_opt(options.flow_num)


async def run(options: 'Values', workflow_id, *tokens_list):
_validate(options, *tokens_list)
Expand Down
9 changes: 9 additions & 0 deletions cylc/flow/scripts/set_outputs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 50,7 @@
from functools import partial
from optparse import Values

from cylc.flow.flow_mgr import validate_flow_opt
from cylc.flow.network.client_factory import get_client
from cylc.flow.network.multi import call_multi
from cylc.flow.option_parsers import (
Expand All @@ -58,6 59,7 @@
)
from cylc.flow.terminal import cli_function


MUTATION = '''
mutation (
$wFlows: [WorkflowID]!,
Expand Down Expand Up @@ -99,7 101,14 @@ def get_option_parser() -> COP:
return parser


def _validate(options: 'Values', *task_globs: str) -> None:
"""Check combination of options and task globs is valid."""
validate_flow_opt(options.flow_num)


async def run(options: 'Values', workflow_id: str, *tokens_list) -> None:

_validate(options, *tokens_list)
pclient = get_client(workflow_id, timeout=options.comms_timeout)

mutation_kwargs = {
Expand Down
3 changes: 2 additions & 1 deletion cylc/flow/scripts/trigger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 100,8 @@ def get_option_parser() -> COP:
"--flow", action="append", dest="flow", metavar="FLOW",
help=f"Assign the triggered task to all active flows ({FLOW_ALL});"
f" no flow ({FLOW_NONE}); a new flow ({FLOW_NEW});"
f" or a specific flow (e.g. 2). The default is {FLOW_ALL}."
" or a specific integer flow (e.g. 2). The default is"
f" {FLOW_ALL}."
" Reuse the option to assign multiple specific flows."
)

Expand Down
7 changes: 7 additions & 0 deletions tests/unit/scripts/test_hold.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 32,13 @@
'opts, task_globs, expected_err',
[
(Opts(), ['*'], None),
(Opts(flow_num=None), ['*'], None),
(Opts(flow_num=2), ['*'], None),
(
Opts(flow_num='cat'),
['*'],
(InputError, "--flow=cat: value must be integer")
),
(Opts(hold_point_string='2'), [], None),
(
Opts(hold_point_string='2'),
Expand Down
7 changes: 7 additions & 0 deletions tests/unit/scripts/test_release.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 33,13 @@
[
(Opts(), ['*'], None),
(Opts(release_all=True), [], None),
(Opts(flow_num=None), ['*'], None),
(Opts(flow_num=2), ['*'], None),
(
Opts(flow_num='cat'),
['*'],
(InputError, "--flow=cat: value must be integer")
),
(
Opts(release_all=True),
['*'],
Expand Down