Skip to content
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

Rationalize the CLI command set. #1249

Closed
8 of 9 tasks
hjoliver opened this issue Dec 3, 2014 · 18 comments
Closed
8 of 9 tasks

Rationalize the CLI command set. #1249

hjoliver opened this issue Dec 3, 2014 · 18 comments

Comments

@hjoliver
Copy link
Member

hjoliver commented Dec 3, 2014

The cylc CLI command set has gradually grown over a long period, so it could do with a rethink. Some commands could be removed, renamed, or combined with others. For instance, should all commands that retrieve information from a suite daemon be combined into one, or should they at least have a common prefix that is not shared by commands that retrieve information from parsing suite definitions? Is the "set-verbosity" command ever used? (suite reload can do "set-xxx" jobs, although it could be made more efficient - not reload task definitions if task definitions haven't changed).

(Edit: Bruno, added task list)

@matthewrmshin
Copy link
Contributor

(I'd imagine that we'll do this as part of the communication API refactor.)

@hjoliver
Copy link
Member Author

hjoliver commented Dec 3, 2014

(I'd imagine that we'll do this as part of the communication API refactor.)

Well, to me this is not obviously related to that. Some CLI commands call the high level suite daemon info and control API, but that is largely independent of the underlying communication protocols that are the main focus of that refactor (i.e. moving from Pyro to RESTful HTTP); and many commands don't interact with suite daemons at all.

@matthewrmshin
Copy link
Contributor

OK. I guess I misunderstood this issue somewhat. However, for those commands that do interact with the suite daemon, I would expect us to just put a very thin CLI on top, once we have got #1250 sorted.

@hjoliver
Copy link
Member Author

hjoliver commented Dec 3, 2014

Agreed.

@hjoliver
Copy link
Member Author

[meeting] - do this after #1250, at least for the commands that do interact with suite daemons.

@kinow
Copy link
Member

kinow commented Apr 5, 2019

cylc-make-docs had issues with setup.py, as it was trying to generate the documentation using a location relative to the cylc-make-docs script. That script is now in some venv/bin/cylc-make-docs for virtual environments. So that command simply cannot work that way.

Instead now we will be able to just use Sphinx integration with setuptools 👍 Marked as done in this check list.

@matthewrmshin
Copy link
Contributor

(Only item left is cylc documentation.)

@kinow
Copy link
Member

kinow commented Jul 29, 2019

I'd be keen to remove it, and perhaps add a note to --help. I think the previous command would simply open the browser with the doc url. Fine it others prefer to keep it and just use the new URL

@matthewrmshin
Copy link
Contributor

It appears to have some kind of suite-task documentation functionality. In the world of Cylc 8, I guess this can easily be absorbed into the UI?

@hjoliver
Copy link
Member Author

Yes, cylc doc can be removed. It was a just a quick way to fire up a browser on a URL parsed from global (Cylc docs) or suite config (suite or task URL metadata). That should not be needed any more.

@hjoliver
Copy link
Member Author

(Only item left is cylc documentation.)

Well... the bullet points and check boxes were added later, I think. The original intent of this issue was to review the entire command set, which seems overly huge:

~/c/cylc-flow (master|…) $ cylc --help all | wc -l
67

However, not a high priority I guess.

@matthewrmshin matthewrmshin modified the milestones: cylc-8.0a2, cylc-8.0.0 Aug 1, 2019
@matthewrmshin
Copy link
Contributor

Agreed not high priority. Should probably get it done for Cylc-8.0.0 though.

@oliver-sanders
Copy link
Member

We can probably get rid of the cylc documentation command now?

@oliver-sanders
Copy link
Member

After discussions on Riot the next two commands to bite the dust are:

  • cylc submit
    • Incompatible with the new architecture.
    • No longer needed due to live reload functionality.
    • Can consider doing something like adding a --reload option to cylc trigger if needed.
  • cylc jobscript
    • Dependent on cylc submit
    • Also deemed unnecessary.
    • Can potentially be re-implemented in a different form if required.

Ping @datamel (cite this issue in your PR).

@oliver-sanders
Copy link
Member

For the record we also removed cylc scp-transfer after a discussion at CylcCon2020

See #3634

@oliver-sanders
Copy link
Member

#3893

@oliver-sanders
Copy link
Member

The following commands to be removed by Cylc8
* checkpoint - #3891
* client - somewhat obsolete since GraphQL, remove or rename to query to match subscribe.
* get-directory - will be made obsolete by #3889
* ls-checkpoints - #3891
* register - will be made obsolete by #3889

This was referenced Oct 29, 2020
@oliver-sanders
Copy link
Member

oliver-sanders commented Aug 4, 2021

I think we have done everything on the list 🎉, CLI well rationalized!

Only one dangling comment:

  • client - somewhat obsolete since GraphQL, remove or rename to query to match subscribe.

Since that comment the client command and some others have been marked as "internal" so don't show up in cylc help or cylc help all output so no longer a big deal.

Not really enough to keep this issue open so closing.

@oliver-sanders oliver-sanders removed this from the cylc-8.0.0 milestone Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants