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.

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

Cylc cli runN fix #4228

Merged
merged 4 commits into from
May 26, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ workflow's public database file was not closed properly.

### Fixes

[#4228](https://github.com/cylc/cylc-flow/pull/4228) - Interacting with a
workflow on the cli using `runN` is now supported.

[#4193](https://github.com/cylc/cylc-flow/pull/4193) - Standard `cylc install`
now correctly installs from directories with a `.` in the name. Symlink dirs
now correctly expands environment variables on the remote. Fixes minor cosmetic
Expand Down
15 changes: 14 additions & 1 deletion cylc/flow/option_parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,26 @@
import logging
from optparse import OptionParser, OptionConflictError, Values, Option
import os
import re
from ansimarkup import parse as cparse
import sys
from typing import Any, Dict, Optional, List

from cylc.flow import LOG, RSYNC_LOG
import cylc.flow.flags
from cylc.flow.loggingutil import CylcLogFormatter
from cylc.flow.terminal import format_shell_examples


def format_shell_examples(string):
"""Put comments in the terminal "dimished" colour."""
return cparse(
re.sub(
r'^(\s*(?:\$[^#]+)?)(#.*)$',
r'\1<dim>\2</dim>',
string,
flags=re.M
)
)


def verbosity_to_opts(verb: int) -> List[str]:
Expand Down
6 changes: 2 additions & 4 deletions cylc/flow/scripts/cylc.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,9 @@
import pkg_resources

from cylc.flow import __version__, iter_entry_points
from cylc.flow.option_parsers import format_shell_examples
from cylc.flow.scripts import cylc_header
from cylc.flow.terminal import (
format_shell_examples,
print_contents
)
from cylc.flow.terminal import print_contents


def get_version(long=False):
Expand Down
37 changes: 25 additions & 12 deletions cylc/flow/terminal.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
from cylc.flow.exceptions import CylcError
from cylc.flow.loggingutil import CylcLogFormatter
from cylc.flow.parsec.exceptions import ParsecError
from cylc.flow.pathutil import get_workflow_run_dir
from cylc.flow.workflow_files import WorkflowFiles


# CLI exception message format
Expand Down Expand Up @@ -74,18 +76,6 @@ def centered(string, width=None):
)


def format_shell_examples(string):
"""Put comments in the terminal "dimished" colour."""
return cparse(
re.sub(
r'^(\s*(?:\$[^#]+)?)(#.*)$',
r'\1<dim>\2</dim>',
string,
flags=re.M
)
)


def print_contents(contents, padding=5, char='.', indent=0):
title_width = max(
len(title)
Expand Down Expand Up @@ -199,6 +189,23 @@ def parse_dirty_json(stdout):
raise ValueError(orig)


def parse_reg(arg: str):
"""Replace runN with true reg name.

Args:
arg (str): flow as entered by user on cli e.g. myflow/runN
"""
arg = arg.rstrip('/')
if not arg.startswith(get_workflow_run_dir('')):
workflow_dir = get_workflow_run_dir(arg)
else:
workflow_dir = arg
run_number = re.search( #type: ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is ok ignoring this mypy...
Unless I am missing something, as far as I can see, mypy thinks that re.search could return none, and therefore can not have group(1) applied. This is called on condition that the arg ends in runN (or runN/) and so should never return none when the symlink is followed.

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't happen but is technically possible, could add an if statement to handle this case.

r'(?:run)(\d*$)',
os.readlink(workflow_dir)).group(1)
return arg.replace(WorkflowFiles.RUN_N, f'run{run_number}')


def cli_function(parser_function=None, **parser_kwargs):
"""Decorator for CLI entry points.

Expand Down Expand Up @@ -226,6 +233,12 @@ def wrapper(*api_args):
list(api_args),
**parser_kwargs
)
# Ensure runN args are replaced with actual run number.
endings = (WorkflowFiles.RUN_N, f'{WorkflowFiles.RUN_N}/')
args = [
parse_reg(cli_arg) if cli_arg.endswith(endings)
else cli_arg for cli_arg in args
]
Comment on lines +237 to +241
Copy link
Member

@oliver-sanders oliver-sanders May 26, 2021

Choose a reason for hiding this comment

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

I'm not very keen on doing this to CLI args arbitrarily, however, the parsing of the registration argument is not properly centralised at the moment so there isn't really anywhere better to put it.

Adding a note to #4230 (comment) as we should be getting onto that shortly so can leave until then.

use_color = (
hasattr(opts, 'color')
and (
Expand Down
52 changes: 52 additions & 0 deletions tests/functional/shutdown/20-stop-after-runN-play.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#!/usr/bin/env bash
# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE.
# Copyright (C) NIWA & British Crown (Met Office) & Contributors.
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#-------------------------------------------------------------------------------
# Test workflow shuts down, having been started with cylc play flow/runN

. "$(dirname "$0")/test_header"
#-------------------------------------------------------------------------------
set_test_number 5
#-------------------------------------------------------------------------------

make_rnd_workflow
pushd "${RND_WORKFLOW_SOURCE}" || exit 1

cat > 'flow.cylc' <<__FLOW_CONFIG__
[scheduler]
[[events]]
abort on inactivity = True
inactivity = PT3M
[scheduling]
[[graph]]
R1 = t1
[runtime]
[[t1]]
script = true
__FLOW_CONFIG__

run_ok "${TEST_NAME_BASE}-install" cylc install 2>'/dev/null'
popd || exit 1
run_ok "${TEST_NAME_BASE}-validate" cylc validate "${RND_WORKFLOW_RUNDIR}/runN"
run_ok "${TEST_NAME_BASE}-play" cylc play "${RND_WORKFLOW_NAME}/runN" --pause
LOG="${RND_WORKFLOW_RUNDIR}/run1/log/workflow/log"
run_ok "${TEST_NAME_BASE}-stop" cylc stop --now --now "${RND_WORKFLOW_NAME}/run1"
log_scan "${TEST_NAME_BASE}-log-stop" "${LOG}" 20 1 \
"INFO - Workflow shutting down - REQUEST(NOW-NOW)"
# stop workflow - workflow should already by stopped but just to be safe
cylc stop --max-polls=10 --interval=2 --kill "${RND_WORKFLOW_NAME}/runN" 2>'/dev/null'

purge_rnd_workflow