diff --git a/changes.d/6261.feat.md b/changes.d/6261.feat.md new file mode 100644 index 00000000000..46d135919fd --- /dev/null +++ b/changes.d/6261.feat.md @@ -0,0 +1 @@ +Allow a workflow to be restarted by `cylc vr` even if there are no changes to reinstall. diff --git a/cylc/flow/scripts/validate_reinstall.py b/cylc/flow/scripts/validate_reinstall.py index 1385ad20bc1..bb09a727381 100644 --- a/cylc/flow/scripts/validate_reinstall.py +++ b/cylc/flow/scripts/validate_reinstall.py @@ -46,6 +46,8 @@ if TYPE_CHECKING: from optparse import Values +from ansimarkup import parse as cparse + from cylc.flow import LOG from cylc.flow.exceptions import ( ContactFileExists, @@ -74,7 +76,7 @@ from cylc.flow.scripts.reload import ( run as cylc_reload ) -from cylc.flow.terminal import cli_function +from cylc.flow.terminal import cli_function, is_terminal from cylc.flow.workflow_files import detect_old_contact_file CYLC_ROSE_OPTIONS = COP.get_cylc_rose_options() @@ -87,6 +89,10 @@ modify={'cylc-rose': 'validate, install'} ) +_input = input # to enable testing + +NO_CHANGES_STR = 'No changes to reinstall' + def get_option_parser() -> COP: parser = COP( @@ -189,7 +195,7 @@ async def vr_cli( # Force on the against_source option: options.against_source = True - # Run cylc validate + # Run "cylc validate" log_subcommand('validate --against-source', workflow_id) await cylc_validate(parser, options, workflow_id) @@ -197,26 +203,46 @@ async def vr_cli( delattr(options, 'against_source') delattr(options, 'is_validate') + # Run "cylc reinstall" log_subcommand('reinstall', workflow_id) reinstall_ok = await cylc_reinstall( - options, workflow_id, + options, + workflow_id, [], print_reload_tip=False ) if not reinstall_ok: - LOG.warning( - 'No changes to source: No reinstall or' - f' {"reload" if workflow_running else "play"} required.' - ) - return False - - # Run reload if workflow is running or paused: + if ( + not workflow_running + and is_terminal() + and not options.skip_interactive + ): + # there are no changes to install but the workflow isn't running + # => ask the user if they want to restart it anyway + usr = None + while usr not in ['y', 'n']: + LOG.warning(NO_CHANGES_STR) + usr = _input( + cparse('Restart anyway? [y/n]: ') + ).lower() + if usr == 'n': + return False + else: + # the are no changes to install and the workflow is running + # => there is nothing for us to do here + LOG.warning( + f'{NO_CHANGES_STR}: No reinstall or' + f' {"reload" if workflow_running else "play"} required.' + ) + return False + + # Run "cylc reload" (if workflow is running or paused) if workflow_running: log_subcommand('reload', workflow_id) await cylc_reload(options, workflow_id) return True - # run play anyway, to play a stopped workflow: + # Run "cylc play" (if workflow is stopped) else: set_timestamps(LOG, options.log_timestamp) cleanup_sysargv( diff --git a/tests/integration/scripts/__init__.py b/tests/integration/scripts/__init__.py new file mode 100644 index 00000000000..763e078d6a6 --- /dev/null +++ b/tests/integration/scripts/__init__.py @@ -0,0 +1,15 @@ +# 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 . diff --git a/tests/integration/scripts/conftest.py b/tests/integration/scripts/conftest.py new file mode 100644 index 00000000000..b76ba422cb6 --- /dev/null +++ b/tests/integration/scripts/conftest.py @@ -0,0 +1,56 @@ +#!/usr/bin/env python3 + +# 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 . + +from types import SimpleNamespace +from uuid import uuid1 + +import pytest + +from cylc.flow.workflow_files import WorkflowFiles + +from ..utils.flow_writer import flow_config_str + + +@pytest.fixture +def one_src(tmp_path, one_conf): + src_dir = tmp_path + (src_dir / 'flow.cylc').write_text(flow_config_str(one_conf)) + (src_dir / 'rose-suite.conf').touch() + return SimpleNamespace(path=src_dir) + + +@pytest.fixture +def one_run(one_src, test_dir, run_dir): + w_run_dir = test_dir / str(uuid1()) + w_run_dir.mkdir() + (w_run_dir / 'flow.cylc').write_text( + (one_src.path / 'flow.cylc').read_text() + ) + (w_run_dir / 'rose-suite.conf').write_text( + (one_src.path / 'rose-suite.conf').read_text() + ) + install_dir = (w_run_dir / WorkflowFiles.Install.DIRNAME) + install_dir.mkdir(parents=True) + (install_dir / WorkflowFiles.Install.SOURCE).symlink_to( + one_src.path, + target_is_directory=True, + ) + return SimpleNamespace( + path=w_run_dir, + id=str(w_run_dir.relative_to(run_dir)), + ) diff --git a/tests/integration/test_reinstall.py b/tests/integration/scripts/test_reinstall.py similarity index 93% rename from tests/integration/test_reinstall.py rename to tests/integration/scripts/test_reinstall.py index b79116828f6..60b0f077f6b 100644 --- a/tests/integration/test_reinstall.py +++ b/tests/integration/scripts/test_reinstall.py @@ -19,8 +19,6 @@ import asyncio from contextlib import asynccontextmanager from pathlib import Path -from types import SimpleNamespace -from uuid import uuid1 import pytest @@ -39,7 +37,7 @@ WorkflowFiles, ) -from .utils.entry_points import EntryPointWrapper +from ..utils.entry_points import EntryPointWrapper ReInstallOptions = Options(reinstall_gop()) @@ -66,32 +64,6 @@ def non_interactive(monkeypatch): ) -@pytest.fixture -def one_src(tmp_path): - src_dir = tmp_path - (src_dir / 'flow.cylc').touch() - (src_dir / 'rose-suite.conf').touch() - return SimpleNamespace(path=src_dir) - - -@pytest.fixture -def one_run(one_src, test_dir, run_dir): - w_run_dir = test_dir / str(uuid1()) - w_run_dir.mkdir() - (w_run_dir / 'flow.cylc').touch() - (w_run_dir / 'rose-suite.conf').touch() - install_dir = (w_run_dir / WorkflowFiles.Install.DIRNAME) - install_dir.mkdir(parents=True) - (install_dir / WorkflowFiles.Install.SOURCE).symlink_to( - one_src.path, - target_is_directory=True, - ) - return SimpleNamespace( - path=w_run_dir, - id=str(w_run_dir.relative_to(run_dir)), - ) - - async def test_rejects_random_workflows(one, one_run): """It should only work with workflows installed by cylc install.""" with pytest.raises(WorkflowFilesError) as exc_ctx: diff --git a/tests/integration/scripts/test_validate_reinstall.py b/tests/integration/scripts/test_validate_reinstall.py new file mode 100644 index 00000000000..3a62219dff6 --- /dev/null +++ b/tests/integration/scripts/test_validate_reinstall.py @@ -0,0 +1,126 @@ +#!/usr/bin/env python3 + +# 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 . + +from cylc.flow.option_parsers import Options +from cylc.flow.scripts.validate_reinstall import ( + get_option_parser as vr_gop, + vr_cli, +) + +ValidateReinstallOptions = Options(vr_gop()) + + +def answer_prompts(monkeypatch, *responses): + """Hardcode responses to "cylc vr" interactive prompts.""" + # make it look like we are running this command in a terminal + monkeypatch.setattr( + 'cylc.flow.scripts.validate_reinstall.is_terminal', + lambda: True + ) + monkeypatch.setattr( + 'cylc.flow.scripts.reinstall.is_terminal', + lambda: True + ) + + # patch user input + count = -1 + + def _input(prompt): + nonlocal count, responses + count += 1 + print(prompt) # send the prompt to stdout for testing + return responses[count] + + monkeypatch.setattr( + 'cylc.flow.scripts.validate_reinstall._input', + _input, + ) + + +async def test_prompt_for_running_workflow_with_no_changes( + monkeypatch, + caplog, + capsys, + log_filter, + one_run, + capcall, +): + """It should reinstall and restart the workflow with no changes. + + See: https://github.com/cylc/cylc-flow/issues/6261 + + We hope to get users into the habit of "cylc vip" to create a new run, + and "cylc vr" to contine an old one (picking up any new changes in the + process). + + This works fine, unless there are no changes to reinstall, in which case + the "cylc vr" command exits (nothing to do). + + The "nothing to reinstall" situation can be interpreted two ways: + 1. Unexpected error, the user expected there to be something to reinstall, + but there wasn't. E.g, they forgot to press save. + 2. Unexpected annoyance, I wanted to restart the workflow, just do it. + + To handle this we explain that there are no changes to reinstall and + prompt the user to see if they want to press save or restart the workflow. + """ + # disable the clean_sysargv logic (this interferes with other tests) + cleanup_sysargv_calls = capcall( + 'cylc.flow.scripts.validate_reinstall.cleanup_sysargv' + ) + + # answer "y" to prompt + answer_prompts(monkeypatch, 'y') + + # attempt to restart it with "cylc vr" + ret = await vr_cli( + vr_gop(), ValidateReinstallOptions(), one_run.id + ) + # the workflow should reinstall + assert ret + + # the user should have been warned that there were no changes to reinstall + assert log_filter(caplog, contains='No changes to reinstall') + + # they should have been presented with a prompt + # (to which we have hardcoded the response "y") + assert 'Restart anyway?' in capsys.readouterr()[0] + + # the workflow should have restarted + assert len(cleanup_sysargv_calls) == 1 + + +async def test_reinstall_abort( + monkeypatch, + capsys, + log_filter, + one_run, +): + """It should abort reinstallation according to user prompt.""" + # answer 'n' to prompt + answer_prompts(monkeypatch, 'n') + + # attempt to restart it with "cylc vr" + ret = await vr_cli( + vr_gop(), ValidateReinstallOptions(), one_run.id + ) + assert ret is False + + # they should have been presented with a prompt + # (to which we have hardcoded the response "n") + assert 'Restart anyway?' in capsys.readouterr()[0]