Skip to content

Commit

Permalink
vr: restart the workflow with no changes according to user prompt
Browse files Browse the repository at this point in the history
* Closes #6261
* If there are no changes to reinstall AND the workflow is stopped,
  prompt the user to see whether they want to restart it anyway.
* This makes `cylc vr` more useful as the "I want to restart my
  workflow" command.
* But it also ensures that they are aware if no changes are present
  as they might have forgotten to press save or run the command on
  the wrong workflow or whatever.
  • Loading branch information
oliver-sanders committed Oct 22, 2024
1 parent df89a77 commit 5af333d
Show file tree
Hide file tree
Showing 6 changed files with 205 additions and 40 deletions.
1 change: 1 addition & 0 deletions changes.d/6354.feat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix an issue that could cause clock-expired tasks to be erroneously retried if "execution retry delays" were configured.
48 changes: 37 additions & 11 deletions cylc/flow/scripts/validate_reinstall.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand All @@ -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(
Expand Down Expand Up @@ -189,34 +195,54 @@ 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)

# Unset options that do not apply after validation:
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('<bold>Restart anyway?</bold> [y/n]: ')
).lower()
if usr == 'n':
return False

Check warning on line 229 in cylc/flow/scripts/validate_reinstall.py

View check run for this annotation

Codecov / codecov/patch

cylc/flow/scripts/validate_reinstall.py#L229

Added line #L229 was not covered by tests
else:
# the are no changes to install and the workflow is running
# => there is nothing for us to do here
LOG.warning(

Check warning on line 233 in cylc/flow/scripts/validate_reinstall.py

View check run for this annotation

Codecov / codecov/patch

cylc/flow/scripts/validate_reinstall.py#L233

Added line #L233 was not covered by tests
f'{NO_CHANGES_STR}: No reinstall or'
f' {"reload" if workflow_running else "play"} required.'
)
return False

Check warning on line 237 in cylc/flow/scripts/validate_reinstall.py

View check run for this annotation

Codecov / codecov/patch

cylc/flow/scripts/validate_reinstall.py#L237

Added line #L237 was not covered by tests

# 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(
Expand Down
15 changes: 15 additions & 0 deletions tests/integration/scripts/__init__.py
Original file line number Diff line number Diff line change
@@ -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 <http://www.gnu.org/licenses/>.
56 changes: 56 additions & 0 deletions tests/integration/scripts/conftest.py
Original file line number Diff line number Diff line change
@@ -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 <http://www.gnu.org/licenses/>.

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)),
)
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
import asyncio
from contextlib import asynccontextmanager
from pathlib import Path
from types import SimpleNamespace
from uuid import uuid1

import pytest

Expand All @@ -39,7 +37,7 @@
WorkflowFiles,
)

from .utils.entry_points import EntryPointWrapper
from ..utils.entry_points import EntryPointWrapper

ReInstallOptions = Options(reinstall_gop())

Expand All @@ -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:
Expand Down
95 changes: 95 additions & 0 deletions tests/integration/scripts/test_validate_reinstall.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
#!/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 <http://www.gnu.org/licenses/>.

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())


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 habbit 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 interpretted 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 all prompts
def _input(prompt):
print(prompt)
return 'y'

monkeypatch.setattr(
'cylc.flow.scripts.validate_reinstall._input',
_input,
)

# 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
)

# 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

0 comments on commit 5af333d

Please sign in to comment.