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

Allow cylc clean to use runN #4872

Merged
merged 1 commit into from
May 18, 2022
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -100,6 +100,9 @@ an invalid `--stopcp` would corrupt the workflow database. Also fix
inconsistency between how `[scheduling]stop after cycle point` was handled
on reload/restart compared to the other cycle point settings.

[#4872](https://github.com/cylc/cylc-flow/pull/4872) - Fix bug preventing
`cylc clean <workflow_name>/runN` from working.

-------------------------------------------------------------------------------
## __cylc-8.0rc2 (<span actions:bind='release-date'>Released 2022-03-23</span>)__

Expand Down
30 changes: 18 additions & 12 deletions cylc/flow/workflow_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -766,10 +766,14 @@ def init_clean(reg: str, opts: 'Values') -> None:
scheduler filesystem and remote hosts.

Args:
reg: Workflow name.
reg: Workflow name/ID.
opts: CLI options object for cylc clean.
"""
local_run_dir = Path(get_workflow_run_dir(reg))
with suppress(InputError):
local_run_dir, reg = infer_latest_run(
local_run_dir, implicit_runN=False, warn_runN=False
)
try:
_clean_check(opts, reg, local_run_dir)
except FileNotFoundError as exc:
Expand Down Expand Up @@ -1239,20 +1243,21 @@ def check_reserved_dir_names(name: Union[Path, str]) -> None:
def infer_latest_run_from_id(workflow_id: str) -> str:
run_dir = Path(get_workflow_run_dir(workflow_id))
_, reg = infer_latest_run(run_dir)
return str(reg)
return reg


def infer_latest_run(
path: Path,
implicit_runN: bool = True,
) -> Tuple[Path, Path]:
warn_runN: bool = True,
) -> Tuple[Path, str]:
"""Infer the numbered run dir if the workflow has a runN symlink.

Warns users that explicit use of runN is unnessary.
Args:
path: Absolute path to the workflow dir, run dir or runN dir.
implicit_runN: If True, add runN on the end of the path if the path
doesn't include it.
warn_runN: If True, warn that explicit use of runN is unnecessary.

Returns:
path: Absolute path of the numbered run dir if applicable, otherwise
Expand All @@ -1265,20 +1270,21 @@ def infer_latest_run(
"""
cylc_run_dir = get_cylc_run_dir()
try:
reg = path.relative_to(cylc_run_dir)
reg = str(path.relative_to(cylc_run_dir))
except ValueError:
raise ValueError(f"{path} is not in the cylc-run directory")
if not path.exists():
raise InputError(
f'Workflow ID not found: {reg}\n(Directory not found: {path})'
)
if path.name == WorkflowFiles.RUN_N:
LOG.warning(
f"Explicit use of {WorkflowFiles.RUN_N} in the Workflow ID is not"
" necessary. It is used automatically to select the latest run"
" number."
)
runN_path = path
if warn_runN:
LOG.warning(
f"You do not need to include {WorkflowFiles.RUN_N} in the "
"workflow ID; Cylc will select the latest run if just the "
"workflow name is used"
)
elif implicit_runN:
runN_path = path / WorkflowFiles.RUN_N
if not os.path.lexists(runN_path):
Expand All @@ -1289,15 +1295,15 @@ def infer_latest_run(
raise WorkflowFilesError(
f"{runN_path} symlink not valid"
)
numbered_run = os.readlink(str(runN_path))
numbered_run = os.readlink(runN_path)
if not re.match(r'run\d+$', numbered_run):
# Note: the link should be relative. This means it won't work for
# cylc 8.0b1 workflows where it was absolute (won't fix).
raise WorkflowFilesError(
f"{runN_path} symlink target not valid: {numbered_run}"
)
path = runN_path.parent / numbered_run
reg = path.relative_to(cylc_run_dir)
reg = str(path.relative_to(cylc_run_dir))
return (path, reg)


Expand Down
33 changes: 33 additions & 0 deletions tests/functional/cylc-clean/04-runN.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#!/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 cleaning latest run using <workflow_name>/runN

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

install_workflow "$TEST_NAME_BASE" basic-workflow true

exists_ok "${WORKFLOW_RUN_DIR}/run1"
exists_ok "${WORKFLOW_RUN_DIR}/runN"

run_ok "${TEST_NAME_BASE}-clean" cylc clean "${WORKFLOW_NAME}/runN"

exists_fail "${WORKFLOW_RUN_DIR}/run1"
exists_fail "${WORKFLOW_RUN_DIR}/runN"

purge
29 changes: 17 additions & 12 deletions tests/unit/test_workflow_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ def test_infer_latest_run(
run_dir.mkdir(parents=True)

path: Path = Path(path.format(cylc_run=cylc_run_dir))
expected = (cylc_run_dir / expected_reg, Path(expected_reg))
expected = (cylc_run_dir / expected_reg, expected_reg)

# Test
assert infer_latest_run(path, implicit_runN) == expected
Expand All @@ -303,18 +303,23 @@ def test_infer_latest_run(
assert infer_latest_run(path) == expected


def test_infer_latest_run_warns_for_runN(caplog, tmp_run_dir):
@pytest.mark.parametrize('warn_arg', [True, False])
def test_infer_latest_run_warns_for_runN(
warn_arg: bool,
caplog: pytest.LogCaptureFixture,
log_filter: Callable,
tmp_run_dir: Callable,
):
"""Tests warning is produced to discourage use of /runN in workflow_id"""
caplog.set_level(logging.WARNING, 'log')
(tmp_run_dir() / 'run1').mkdir()
runN_path = tmp_run_dir() / 'runN'
runN_path.symlink_to('run1')
infer_latest_run(runN_path)
warning_raised = False
for x in caplog.messages:
if re.match("Explicit use of runN in the Workflow ID", x):
warning_raised = True
assert warning_raised is True
infer_latest_run(runN_path, warn_runN=warn_arg)
filtered_log = log_filter(
caplog, level=logging.WARNING,
contains="You do not need to include runN in the workflow ID"
)
assert filtered_log if warn_arg else not filtered_log


@pytest.mark.parametrize(
Expand All @@ -329,7 +334,7 @@ def test_infer_latest_run_warns_for_runN(caplog, tmp_run_dir):
)
def test_infer_latest_run__bad(
reason: str,
error_type: Exception,
error_type: Type[Exception],
tmp_run_dir: Callable,
) -> None:
# -- Setup --
Expand Down Expand Up @@ -360,9 +365,9 @@ def test_infer_latest_run__bad(
else:
raise ValueError(reason)
# -- Test --
with pytest.raises(error_type) as exc:
with pytest.raises(error_type) as excinfo:
infer_latest_run(run_dir)
assert str(exc.value) == err_msg
assert str(excinfo.value) == err_msg


@pytest.mark.parametrize(
Expand Down