cylc play (restart): fix bug affecting run host reinvocation after interactive upgrade

[MetRonnie]

Fixes a bug not recorded in an issue. I think this bug has been present since the introduction of the interactive upgrade mechanism (#5074)

TL;DR - remote reinvocation would not work after interactively upgrading a workflow to a new Cylc version on the CLI.

Steps to reproduce

With run hosts configured in global.cylc

[scheduler]
    [[run hosts]]
        available = <server1>, ...

Start a workflow and stop it, then fiddle the workflow DB to make it look like it was run with an older version:

sqlite3 wflow/.service/db 'UPDATE workflow_params SET value="8.1.7" WHERE key="cylc_version"'

Now restart it with cylc play, and type 'y' at the interactive upgrade prompt. When it re-invokes on the run host, you should see an error

This workflow was previously run with 8.1.7.
This version of Cylc is 8.3.4.dev
Use "--upgrade" to upgrade the workflow.
ERROR: ERROR: command terminated by signal 1: ssh ...

Check List

• I have read CONTRIBUTING.md and added my name as a Code Contributor.
• Contains logically grouped changes (else tidy your branch by rebase).
• Does not contain off-topic changes (use other PRs for other changes).
• No dependency changes
• Tests are included
• Changelog entry included if this is a change that can affect users
• No docs entry needed
• If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

[MetRonnie]

Test failures just macos functional issues and flaky integration test fixed by #6257

[wxtim]

👍🏼

[oliver-sanders]

FYI, I purposefully did not propagate the options object down the call chain. IMO it's a good idea to abstract out the CLI interfaces higher up in the call chain to keep the underlying code more abstract. The more you propagate CLI logic down the call stack, the harder it is to, for example, open a Python API later, or create a wrapper command, you also need to mock more in order to test and typing is less effective too (we know we're getting a Values object, but we don't know whether it has an upgrade option.

Not a blocker, but better before IMO.

[MetRonnie]

I think ideally we want to only alter options.upgrade in the case of an interactive upgrade, so the simplest way of doing this is passing in the options object to _version_check.

It is a shame about the type-looseness of it, but at least it is relatively easy to test these functions by importing and using RunOptions (no need for mocking).

cylc-flow/cylc/flow/option_parsers.py

Lines 569 to 570 in 6eb8f61

	class Options:
	"""Wrapper to allow Python API access to optparse CLI functionality.

[oliver-sanders]

options.update = _version_check(db_file, options.upgrade, options.downgrade) would have done it?

[MetRonnie]

There are pathways it returns True without interactive upgrade