Config change.stop after cycle point

[wxtim]

These changes close #3694 : Implement a [scheduling]stop after cycle point configuration to match the cylc run --stop-cycle-point=POINT cli option.

Requirements 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).

• Appropriate tests are included (unit and/or functional).

• Appropriate change log entry included.

• Appropriate Auto-doc string added for the command reference.
• Documentation PR.

• No dependency changes.

[oliver-sanders]

Can this be simplified along the lines of:

if not self.options.stopcp:
    # doesn't matter if stop after cycle point is None
    self.options.stopcp = self.config.cfg['scheduling']['stop after cycle point']
if self.options.stopcp:
    self.pool.set_stop_point(get_point(self.options.stopcp))

?

[wxtim]

Not really: Your suggestion would never allow the command line to over-ride the config: I'm not against tidying or commenting the logic though. It's a bit wierd.

Also, not having 'stop after cycle point' in self.config.cfg['scheduling'] would mean putting this in a try:/except: in case the config item was blank(I mean, I could set a default, but that seems weird in this instance).

[wxtim]

Sources of stopcp by priority

• Command line
• Database
• Config

We can simplify the logic by creating a temporary variable to store our stopcp in, and only overwriting the self.options.stopcp at the end of the process.

We also need to check that --ignore-stop-after-cycle-point works.

I wish that this were an easily unit-testable bit.

Test Cases

Command Line Set	Database Set	Config Set	How
✔️			run or restart; check stopped correctly
✔️	✔️		run with config set; restart with command line set; check finished at CLI stop point
✔️		✔️	run with CLI stop sooner than config; ensure has stopped at CLI stop point
✔️	✔️	✔️	run with config set, stop with cylc stop; restart with command line set
	✔️		run with CLI, stop artificially; restart without CLI set; check flow stopped at stop point
		✔️	run with config; check stopped at right place
	✔️	✔️	run with stop point, stop with cylc stop in suite; sed mod the config stop point restart; check finish at db stop point
			Not a test case

Additionally we should test each of these cases with a restart with and without the --ignore-stop-cycle-point cli option.

[oliver-sanders]

Not really: Your suggestion would never allow the command line to over-ride the config

I think it would, the config value is only read if the CLI opt is not set.

[MetRonnie]

I would have thought a unit test for this method would be better suited than a functional test (quicker to run; we only need to test the logic in the method, otherwise the test corresponding to set_stop_point() should handle the rest? (assuming such a test exists))

[wxtim]

I considered it, but because it's a method of the class it'd require some in un-fun mocking. However it'd be nicer, and I like the idea of working out how.

[MetRonnie]

Looks good apart from question of unit vs functional test (up to you). However still needs a change log entry and maybe a cylc-doc PR?

[wxtim]

Looks good apart from ... and a cylc-doc PR?

I'm not sure about a cylc-doc PR.

I have added some detail to the desc field in the config which should go into the reference section of the documentation.

I don't think there is value in copying that somewhere else. I can't remember what was behind the inclusion of the stop after cycle point item and the workshop notes don't lead me to enlightenment.

What is the use-case for this config item @oliver-sanders? If I understood that I could probably do something in src/tutorial/scheduling/further_scheduling. Otherwise further changes to cylc-doc seem like duplication.

[oliver-sanders]

Don't think this needs adding to the user guide or anything. The best documentation would likely be to note the CLI opt referencing this config (and vice versa).