cylc play #4040

MetRonnie · 2021-01-20T14:14:47Z

These changes partially address #3897

Replace cylc run and cylc restart with cylc play.
Remove the START_POINT optional positional argument.

Tidy up the aliases for cycle points options, so only these are used:

--initial-cycle-point, --icp
--final-cycle-point, --fcp
--start-cycle-point, --stopcp
--stop-cycle-point, --startcp

Note: Running cylc play on a workflow that has previously been run now has the effect of cylc restart. In order to do a cold start, you will have to either run cylc clean or remove the private & public databases before running cylc play (a function for the latter has been added to test_header for the functional tests)

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 included?
Appropriate change log entry included.
(master branch) Will open a documentation PR after cylc pause is implemented
No dependency changes.

MetRonnie · 2021-01-29T17:36:27Z

https://github.com/cylc/cylc-flow/pull/4040/files#annotation_899603301

cylc/flow/scheduler.py#L1358
Added line #L1358 was not covered by tests

This is executed as part of tests/functional/restart/34-auto-restart-basic.t, but doesn't run on Actions as it requires shared filesystem

Add function for deleting DB for cold start

Removed: --ict (icp) --initial-point (icp) --until (fcp) --final-point (fcp) --start-point (startcp) --stop-point (stopcp)

hjoliver · 2021-02-01T19:26:12Z

Brilliant to have this ready to go. Should it be un-Drafted now @MetRonnie ?

MetRonnie · 2021-02-02T10:05:51Z

Still some tests that I should run/add to this

MetRonnie · 2021-02-02T18:49:33Z

@hjoliver Should be ready now

Also remove the part on --icp=ignore, --fcp=ignore etc. Will need to revisit this later

oliver-sanders

Looks good to me, should save the remaining initial/final/start/stop questions for #4062 to avoid blocking this PR.

A few small comments...

oliver-sanders · 2021-02-08T11:33:18Z

cylc/flow/scheduler_cli.py

+    # Install $PWD/flow.cylc as $(basename $PWD) and start it.
+    $ cylc play


I think this example should bite the dust now:

Suggested change

# Install $PWD/flow.cylc as $(basename $PWD) and start it.

$ cylc play

So I should make the [REG] arg non-optional?

Yes please, it no longer makes sense since the change from registration to installation.

This feature will now only work if you are in the run directory for a suite which has either been written there or cylc installed and will fail in unhelpful ways otherwise.

cylc/flow/scheduler_cli.py

tests/functional/cylc-play/00-cycle-point-opts.t

tests/functional/restart/51-ignore-final-point.t

MetRonnie · 2021-02-09T12:36:11Z

Tests re-run timed out. but I ran them locally and they all pass

hjoliver · 2021-02-09T18:52:37Z

The same test batch has suddenly started timing out on #4035, after rebase. There might be a test broken by some recent change on master (if this branch is up to date too?)

MetRonnie · 2021-02-10T10:06:15Z

I think it's just going to need a longer timeout on GH Actions, that's all. It's running the whole test battery in that job, including the flakyfunctional, and possibly the poll tests take longer than tcp.

Having said that, I've looked through the timed out test runs, and it seems there's more often than not a strange delay at the last couple of tests. Not sure why

[17:33:28] tests/f/events/11-cycle-task-event-job-logs-retrieve.t ..... ok   314944 ms ( 0.02 usr  0.03 sys + 161.12 cusr 18.52 csys = 179.69 CPU)
[17:39:09] tests/f/remote/03-polled-task-started.t ..... ok  1632815 ms ( 0.30 usr  1.48 sys + 876.69 cusr 105.05 csys = 983.52 CPU)

~~By the way, the time in ms seems to be 10 times too high?~~ No, it really is taking that long!

oliver-sanders · 2021-02-10T17:29:04Z

Yeah that's not a good sign, if a such a simple test takes so long then something is likely going badly wrong. Sadly there isn't enough evidence to tell what.

I've been stabilising a few tests and added a default workflow timeout in a branch I'm working on at the moment which might potentially help but I'm not sure.

I've seen to outrageously long test runs on polling systems though not as bad as that!

cylc/flow/scheduler_cli.py

hjoliver · 2021-02-15T08:14:58Z

cylc/flow/scheduler_cli.py

+A "restart" continues the workflow from the earliest cycle point that was
+incomplete when shutdown occurred. Tasks recorded as submitted or running are


Suggested change

A "restart" continues the workflow from the earliest cycle point that was

incomplete when shutdown occurred. Tasks recorded as submitted or running are

A "restart" continues on from the most recent recorded state of the workflow.

Tasks recorded as submitted or running are

I know what you mean, but "continues from the [blah] cycle point ..." could be confusing. We've traditionally described warm starts that way. A restart often involves tasks at multiple cycle points.

hjoliver

Looks good, tests as working 🎉

Still one @oliver-sanders comment to respond to? (on the SuiteServiceFileError exception)

MetRonnie · 2021-02-15T11:52:14Z

Both should be sorted now

oliver-sanders · 2021-02-15T16:09:04Z

cylc/flow/scheduler.py

@@ -172,7 +172,6 @@ class Scheduler:
    contact_data: Optional[dict] = None

    # run options
-    is_restart: bool = False


The idea of specifying everything up here is so that we can see all of the Scheduler attributes in one place. Can always default to None until set later.

oliver-sanders

Another Cylc8 brick in the wall, thanks @MetRonnie!

(Small comments above about an unused code block and removed dataclass attr definition).

Ooh, just spotted, a reference to cylc run has snuck in probs with rebase in cylc.flow.scripts.install#24.

And fix references to cylc run added by other PRs

MetRonnie · 2021-02-15T18:55:31Z

Have amended last commit to address those

hjoliver

Thanks @MetRonnie, good job 🎉

MetRonnie added this to the cylc-8.0b0 milestone Jan 20, 2021

MetRonnie self-assigned this Jan 20, 2021

MetRonnie force-pushed the cylc-play branch 2 times, most recently from a4e9d7f to 999769d Compare January 21, 2021 12:34

MetRonnie marked this pull request as draft January 21, 2021 13:39

MetRonnie force-pushed the cylc-play branch 2 times, most recently from 9b79233 to 838edcb Compare January 21, 2021 21:24

MetRonnie force-pushed the cylc-play branch 2 times, most recently from 4a8517c to bcaf267 Compare January 29, 2021 11:32

MetRonnie force-pushed the cylc-play branch from 91cde48 to ac44add Compare February 1, 2021 11:19

MetRonnie added 8 commits February 1, 2021 12:43

Replace cylc run|restart with cylc play

620fc77

Fix unit & integration tests after introducing cylc play

1b7e72e

cylc play: Update test_header

b7bf859

Add function for deleting DB for cold start

Convert func tests to cylc play (these ones require some care)

aec3e85

Convert func tests to cylc play (trivial)

c4dd9c7

Remove aliases for cli options --icp, --fcp, --startcp & --stopcp

e4e1df0

Removed: --ict (icp) --initial-point (icp) --until (fcp) --final-point (fcp) --start-point (startcp) --stop-point (stopcp)

Rename tests/f/cylc-run -> tests/f/cylc-play

a93e572

Fix/tidy func tests as part of cylc run -> cylc play

7518462

MetRonnie force-pushed the cylc-play branch from ac44add to 3182546 Compare February 1, 2021 13:32

MetRonnie marked this pull request as ready for review February 1, 2021 16:17

MetRonnie requested review from oliver-sanders and hjoliver February 1, 2021 16:17

MetRonnie marked this pull request as draft February 1, 2021 17:30

MetRonnie mentioned this pull request Feb 1, 2021

Tidy a few things #4057

Merged

7 tasks

MetRonnie mentioned this pull request Feb 2, 2021

cli: play|pause|stop #3897

Closed

MetRonnie marked this pull request as ready for review February 2, 2021 18:48

MetRonnie added 3 commits February 5, 2021 17:32

Tidy func test

3c95463

Rephrase warm start part of cylc play docstring

a021031

Also remove the part on --icp=ignore, --fcp=ignore etc. Will need to revisit this later

Merge branch 'master' into cylc-play

2af8aa1

MetRonnie force-pushed the cylc-play branch from 5912b74 to 2af8aa1 Compare February 5, 2021 17:35

oliver-sanders reviewed Feb 8, 2021

View reviewed changes

MetRonnie added 3 commits February 8, 2021 17:02

cylc play: make REG (workflow name) arg required

2c42dc3

Address code review

30e4d67

Merge branch 'master' into cylc-play

1ac705b

oliver-sanders reviewed Feb 10, 2021

View reviewed changes

cylc/flow/scheduler_cli.py Outdated Show resolved Hide resolved

MetRonnie added 3 commits February 12, 2021 12:28

Tidy

600261f

Merge branch 'master' into cylc-play

93362cb

Test --fcp=ignore in unit test

7b76aac

hjoliver reviewed Feb 15, 2021

View reviewed changes

hjoliver approved these changes Feb 15, 2021

View reviewed changes

oliver-sanders reviewed Feb 15, 2021

View reviewed changes

oliver-sanders approved these changes Feb 15, 2021

View reviewed changes

Address code review

e826bc5

And fix references to cylc run added by other PRs

MetRonnie force-pushed the cylc-play branch from bd6241e to e826bc5 Compare February 15, 2021 18:54

hjoliver approved these changes Feb 15, 2021

View reviewed changes

hjoliver merged commit 9d65b9a into cylc:master Feb 15, 2021

MetRonnie deleted the cylc-play branch February 16, 2021 11:17

MetRonnie linked an issue Feb 17, 2021 that may be closed by this pull request

cli: play|pause|stop #3897

Closed

MetRonnie mentioned this pull request Feb 17, 2021

Fix missing nightly build in version links cylc/cylc-doc#200

Merged

MetRonnie mentioned this pull request Feb 24, 2021

Cylc play/pause cylc/cylc-doc#204

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cylc play #4040

cylc play #4040

MetRonnie commented Jan 20, 2021 •

edited

Loading

MetRonnie commented Jan 29, 2021

hjoliver commented Feb 1, 2021

MetRonnie commented Feb 2, 2021

MetRonnie commented Feb 2, 2021

oliver-sanders left a comment

oliver-sanders Feb 8, 2021

MetRonnie Feb 8, 2021

oliver-sanders Feb 8, 2021

MetRonnie commented Feb 9, 2021

hjoliver commented Feb 9, 2021

MetRonnie commented Feb 10, 2021 •

edited

Loading

oliver-sanders commented Feb 10, 2021

hjoliver Feb 15, 2021

hjoliver left a comment

MetRonnie commented Feb 15, 2021

oliver-sanders Feb 15, 2021

oliver-sanders left a comment •

edited

Loading

MetRonnie commented Feb 15, 2021

hjoliver left a comment

		# Install $PWD/flow.cylc as $(basename $PWD) and start it.
		$ cylc play

		A "restart" continues the workflow from the earliest cycle point that was
		incomplete when shutdown occurred. Tasks recorded as submitted or running are

cylc play #4040

cylc play #4040

Conversation

MetRonnie commented Jan 20, 2021 • edited Loading

MetRonnie commented Jan 29, 2021

hjoliver commented Feb 1, 2021

MetRonnie commented Feb 2, 2021

MetRonnie commented Feb 2, 2021

oliver-sanders left a comment

Choose a reason for hiding this comment

oliver-sanders Feb 8, 2021

Choose a reason for hiding this comment

MetRonnie Feb 8, 2021

Choose a reason for hiding this comment

oliver-sanders Feb 8, 2021

Choose a reason for hiding this comment

MetRonnie commented Feb 9, 2021

hjoliver commented Feb 9, 2021

MetRonnie commented Feb 10, 2021 • edited Loading

oliver-sanders commented Feb 10, 2021

hjoliver Feb 15, 2021

Choose a reason for hiding this comment

hjoliver left a comment

Choose a reason for hiding this comment

MetRonnie commented Feb 15, 2021

oliver-sanders Feb 15, 2021

Choose a reason for hiding this comment

oliver-sanders left a comment • edited Loading

Choose a reason for hiding this comment

MetRonnie commented Feb 15, 2021

hjoliver left a comment

Choose a reason for hiding this comment

MetRonnie commented Jan 20, 2021 •

edited

Loading

MetRonnie commented Feb 10, 2021 •

edited

Loading

oliver-sanders left a comment •

edited

Loading