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

vr: restart the workflow with no changes according to user prompt #6354

Open
wants to merge 3 commits into
base: 8.3.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions changes.d/6261.feat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow a workflow to be restarted by `cylc vr` even if there are no changes to reinstall.
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 @@
# 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
oliver-sanders marked this conversation as resolved.
Show resolved Hide resolved
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'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
f'{NO_CHANGES_STR}: No reinstall or'
f'{NO_CHANGES_STR}: no'

The STR already says that no reinstall is needed.

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 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 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
Loading