Tweak scan CLI help. #5405

hjoliver · 2023-03-13T06:03:57Z

Improve cylc scan command help
add scheduler PID to the output
~~print a note to stderr to remind that scan only reads contact files by default~~

$ cylc scan 
demo/run9 niwa-1007885.niwa.local:43070 21212  # PIDs new
demo/run7 niwa-1007885.niwa.local:43010 21132
demo/run8 niwa-1007885.niwa.local:43069 21173

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).
Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
Tests are included (or explain why tests are not needed). [Not needed: minor documentation only]
CHANGES.md entry included if this is a change that can affect users
Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

hjoliver · 2023-03-13T06:10:05Z

As an aside, cylc scan -t json just prints contact file content plus a little more static data, e.g. contact file location.

@oliver-sanders was that the intention for -t json? If so, we should document that and disallow use with other options e.g. -t rich -t json which currently doesn't have the expected effect.

oliver-sanders · 2023-03-14T11:26:34Z

@oliver-sanders was that the intention for -t json?

Yes, I think this was inherited from Cylc 7. It's just a better interface for any systems which want to scape this information.

The cylc play command also accepts --format=json for much the same reason (UIS could avoid re-scanning when it starts workflows).

If so, we should document that and disallow use with other options e.g. -t rich -t json which currently doesn't have the expected effect.

???

cylc scan --format=json --format=rich results in --format=rich

In the same way that:

git commit --author=one --author=two results in --author=two

If you repeat an argument, the latter definition is used.

hjoliver · 2023-03-14T22:31:15Z

If you repeat an argument, the latter definition is used.

Duh 🤦

Well, not exclusively true but fair enough.

cylc config -i foo -i bar

What I was getting at was: was the intention to allow all cylc scan output (plain, rich, contact file, ...) to be returned in json format? [arrggh so many "was"s]

It would be reasonable to expect that -t json does that. But all it actually does is return the contents of the contact file (albeit in json format). We should probably rename the option to --contact-file ...

hjoliver · 2023-05-03T02:03:46Z

I've stuck with just documenting what -t json does better:

json: full contact data in JSON format

oliver-sanders · 2023-05-03T09:00:02Z

cylc/flow/scripts/scan.py

+    print(
+        "\nUse `--ping` to attempt to connect to schedulers and clean "
+        "\nup after any that were killed or did not shut down cleanly.",
+        file=stderr
+    )


I think printing this message every time is a bit too obstructive.

Well, it's only printed for non scheduler-contacting cases, and only to stderr. And I got here via a bunch of confused-user questions ("cylc scan says my workflow is running, but stuff isn't working!").

Well, it's only printed for non scheduler-contacting cases

So the default i.e. 99.9% of cases.

I feel that if it is really this important that we add a message to every command, then it should really be the default.

But we don't want to do that in order to keep cylc scan fast.

#5511 possible alternative?

IMO, crashes should be sufficiently unusual that we shouldn't need to bend other commands to accommodate this case. It's mostly a large number of critical bugs reported during Cylc 8 stabilisation which has made this situation common.

This situation is particularly confusing for users when it does happen, because they have a reasonable expectation that cylc scan does not report stopped workflows as running.

However, I'll remove the message. I quite like your proposed #5511 solution.

hjoliver · 2023-05-03T23:01:19Z

@oliver-sanders - this now just clarifies the command help, and adds scheduler PID to the basic scan output. Which makes it easier for users to see if a scheduler really is running, if they need to do that.

One review should do now.

…_too_soon * upstream/master: (70 commits) Tweak scan CLI help. (cylc#5405) Add support for python 3.11 (cylc#5497) tests/u: add parameter ids build(deps): bump pypa/gh-action-pypi-publish from 1.8.5 to 1.8.6 prerequisite: remove unused interface Fix change log dup section. Fix flaky test Update CONTRIBUTING.md Tidy platforms modify CONTRIBUTING.md Bump dev version (cylc#5501) Prepare release 8.1.3 Ignore off-sequence parents, for datastore. (cylc#5495) Avoid duplicate prerequisites from multiple recurrences. (cylc#5466) Update cylc/flow/prerequisite.py prerequsite: remove target_point_strings attribute tests/f: fix events/11 swarm: convert cylc-dev Docker image to run on ubuntu:latest fix changelog Apply prerequisite changes to spawned tasks, on reload & restart (cylc#5334) ...

hjoliver added the small label Mar 13, 2023

hjoliver self-assigned this Mar 13, 2023

hjoliver force-pushed the scan-doc branch 5 times, most recently from 9823ee6 to a2c68b2 Compare March 15, 2023 01:03

hjoliver force-pushed the scan-doc branch from a2c68b2 to f0a7eac Compare May 3, 2023 01:53

hjoliver changed the base branch from 8.1.x to master May 3, 2023 01:55

hjoliver added this to the cylc-8.2.0 milestone May 3, 2023

hjoliver marked this pull request as ready for review May 3, 2023 02:01

hjoliver closed this May 3, 2023

hjoliver reopened this May 3, 2023

oliver-sanders reviewed May 3, 2023

View reviewed changes

oliver-sanders mentioned this pull request May 3, 2023

contact: update modified time for better crash detection #5511

Open

hjoliver added 2 commits May 4, 2023 10:55

Better "cylc scan" command help, and add PID to output.

d0e917f

Update change log.

6f5e3ca

hjoliver force-pushed the scan-doc branch from f0a7eac to 6f5e3ca Compare May 3, 2023 22:56

oliver-sanders approved these changes May 4, 2023

View reviewed changes

oliver-sanders merged commit c05629c into cylc:master May 4, 2023
23 of 24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tweak scan CLI help. #5405

Tweak scan CLI help. #5405

hjoliver commented Mar 13, 2023 •

edited

Loading

hjoliver commented Mar 13, 2023

oliver-sanders commented Mar 14, 2023 •

edited

Loading

hjoliver commented Mar 14, 2023 •

edited

Loading

hjoliver commented May 3, 2023

oliver-sanders May 3, 2023

hjoliver May 3, 2023

oliver-sanders May 3, 2023

oliver-sanders May 3, 2023 •

edited

Loading

hjoliver May 3, 2023 •

edited

Loading

hjoliver commented May 3, 2023

Tweak scan CLI help. #5405

Tweak scan CLI help. #5405

Conversation

hjoliver commented Mar 13, 2023 • edited Loading

hjoliver commented Mar 13, 2023

oliver-sanders commented Mar 14, 2023 • edited Loading

hjoliver commented Mar 14, 2023 • edited Loading

hjoliver commented May 3, 2023

oliver-sanders May 3, 2023

Choose a reason for hiding this comment

hjoliver May 3, 2023

Choose a reason for hiding this comment

oliver-sanders May 3, 2023

Choose a reason for hiding this comment

oliver-sanders May 3, 2023 • edited Loading

Choose a reason for hiding this comment

hjoliver May 3, 2023 • edited Loading

Choose a reason for hiding this comment

hjoliver commented May 3, 2023

hjoliver commented Mar 13, 2023 •

edited

Loading

oliver-sanders commented Mar 14, 2023 •

edited

Loading

hjoliver commented Mar 14, 2023 •

edited

Loading

oliver-sanders May 3, 2023 •

edited

Loading

hjoliver May 3, 2023 •

edited

Loading