From 6606619c3e6b405ef7c7c2452362338eec137a27 Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Fri, 13 May 2022 12:53:00 +0100 Subject: [PATCH] Allow cylc clean to use runN --- CHANGES.md | 3 +++ cylc/flow/workflow_files.py | 30 ++++++++++++++---------- tests/functional/cylc-clean/04-runN.t | 33 +++++++++++++++++++++++++++ tests/unit/test_workflow_files.py | 29 +++++++++++++---------- 4 files changed, 71 insertions(+), 24 deletions(-) create mode 100644 tests/functional/cylc-clean/04-runN.t diff --git a/CHANGES.md b/CHANGES.md index caf3930eef4..965ea669f24 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -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 /runN` from working. + ------------------------------------------------------------------------------- ## __cylc-8.0rc2 (Released 2022-03-23)__ diff --git a/cylc/flow/workflow_files.py b/cylc/flow/workflow_files.py index ab271d2b378..1c055b121b1 100644 --- a/cylc/flow/workflow_files.py +++ b/cylc/flow/workflow_files.py @@ -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: @@ -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 @@ -1265,7 +1270,7 @@ 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(): @@ -1273,12 +1278,13 @@ def infer_latest_run( 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): @@ -1289,7 +1295,7 @@ 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). @@ -1297,7 +1303,7 @@ def infer_latest_run( 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) diff --git a/tests/functional/cylc-clean/04-runN.t b/tests/functional/cylc-clean/04-runN.t new file mode 100644 index 00000000000..3300168ecce --- /dev/null +++ b/tests/functional/cylc-clean/04-runN.t @@ -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 . +# ----------------------------------------------------------------------------- +# Test cleaning latest run using /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 diff --git a/tests/unit/test_workflow_files.py b/tests/unit/test_workflow_files.py index cae7cb91764..29c17ac4a32 100644 --- a/tests/unit/test_workflow_files.py +++ b/tests/unit/test_workflow_files.py @@ -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 @@ -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( @@ -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 -- @@ -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(