-
Notifications
You must be signed in to change notification settings - Fork 94
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
Tweak scan CLI help. #5405
Tweak scan CLI help. #5405
Conversation
As an aside, @oliver-sanders was that the intention for |
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
???
In the same way that:
If you repeat an argument, the latter definition is used. |
Duh 🤦 Well, not exclusively true but fair enough.
What I was getting at was: was the intention to allow all It would be reasonable to expect that |
9823ee6
to
a2c68b2
Compare
I've stuck with just documenting what
|
cylc/flow/scripts/scan.py
Outdated
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 | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think printing this message every time is a bit too obstructive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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!").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it's only printed for non scheduler-contacting cases
So the default i.e. 99.9% of cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@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) ...
cylc scan
command helpprint a note to stderr to remind that scan only reads contact files by defaultCheck List
CONTRIBUTING.md
and added my name as a Code Contributor.setup.cfg
(andconda-environment.yml
if present).CHANGES.md
entry included if this is a change that can affect users?.?.x
branch.