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

cli: command aliases #3896

Closed
oliver-sanders opened this issue Oct 27, 2020 · 20 comments · Fixed by #5229
Closed

cli: command aliases #3896

oliver-sanders opened this issue Oct 27, 2020 · 20 comments · Fixed by #5229
Assignees
Milestone

Comments

@oliver-sanders
Copy link
Member

Consider supporting aliases for common command combinations e.g:

  • install + play
  • reinstall + reload

For inspiration look at git "transactions" e.g:

  • git pull (git fetch + git merge).

And git "aliases" e.g:

  • git config --global alias.co checkout

Also consider ways of exposing these aliases via the UIS so they can be used in the UI as GraphQL mutations.

@oliver-sanders oliver-sanders added this to the 8.x milestone Oct 27, 2020
@oliver-sanders oliver-sanders modified the milestones: cylc-8.x, cylc-8.0rc5 May 10, 2022
@oliver-sanders
Copy link
Member Author

oliver-sanders commented May 10, 2022

Now that we are starting to onboard users the pressure on this one is rapidly rising as users are looking to translate their working patterns to Cylc 8. We now have enough test users running Cylc 8 that we can start polling their opinion on these matters.

Working Patterns

Working Pattern: The user's working pattern aka their "workflow", not to be mistaken with a Cylc workflow.

Here are what I think are the main components of most working patterns:

I've associated each with a letter for illustrative purposes, any combination of these stages is valid. We only really need to support the main use cases but it might be possible to implement a generic framework to allow any/all combinations if we feel that it would actually help.

Rose 2019 Aliases

Alias: For the sake of this issue let's call a CLI command which encompasses multiple functionalities an "alias" to avoid confusion with Rose macros.

Here are the combinations provided by Rose 2019 for comparison:

1) rose suite-run --local-install-only
cylc install  # yay this is easy now

2) rose suite-run --validate-suite-only
cylc validate  # yay this is easy now

3) rose suite-run [--run]
# NOTE: the `vp` bit is not useful any more since play also performs validation)
cylc ivpg

4) rose suite-run --restart
# NOTE: the `vp` bit is not useful any more since play also performs validation)
cylc rvp

5) rose suite-run --reload
cylc ro  # was there a validate step in here anywhere?

6) rose suite-run --new
cylc clean; cylc ivpg

7-12) rose stem ...
# The same as 1-6 but with `rose stem` rather than `cylc install`

Note: Rose commands opened the Gui by default but this could be disabled using --no-gcontrol.

Note: Rose commands supported passing CLI arguments to Cylc using a -- argument separator.

Possible Cylc 8 Aliases

Example combinations to start the debate:

# I would expect this to be the most common working pattern
cylc vip
cylc vip[gte]  # +ui

# the rose-stem equivalent (validate doesn't make sense for rose stem)
cylc sp
cylc sp[gte]  # +ui (tui likely to be a popular choice w/ `rose stem`)

# modified pattern for complex workflows where validation time >> install time
# (is this still a problem, it's been a while since I have tested configuration load times)
cylc ip
cylc ip[gte]  # +ui

# validate & install & reload
cylc rvo  # easy - but the validate is of no use since the damage has already been done by install
cylc vro  # hard - need to read CLI options from the run dir in order to validate the src dir:
          # * rose optional config (contains Rose optional config & template vars specified to `cylc install`)
          # * workflow DB (contains template vars specified to `cylc play`)

Opinion

I think the most important aliases to implement are:

  • vip[gt]
  • sp[gt]
  • vro

To find out for sure we will have to poll users.

Naming the Aliases

The letters I've used are illustrative, we could go with them, however, commands such as vro aren't the most intuitive but what's the alternative?!

# this:
cylc validate-install-play
# is barely better than this:
cylc validate; cylc install; cylc play

We could follow the Rose pattern and make these things tag-on options to install:

$ cylc install --play
$ cylc reinstall --reload

But that doesn't cover cases which start with validate and makes passing CLI arguments tricky.

We could potentially give aliases a prefix such as : e.g. :vip to differentiate them 🤷, ideas?

Technical Hurdles

The biggest barrier is working out how to handle the CLI options e.g. with vip any template variables provided via -s need to be provided to v & p whereas any template variables provided with -S (cylc-rose) need to be provided to v & i. The vip command will have to make this happen.

@oliver-sanders oliver-sanders added the question Flag this as a question for the next Cylc project meeting. label May 11, 2022
@oliver-sanders
Copy link
Member Author

To add to this, today I have been using the following pattern which also incorporates cylc clean:

cylc vcipt  # validate - clean - install - play - tui

... as part of a rapid workflow development cycle. In this case I had no reason to keep old runs once I was done with them so was cleaning as I went to keep things cleaner. I wasn't using --no-run-name as the runN wasn't an encumbrance, however, could have.

@oliver-sanders
Copy link
Member Author

Proposal

  1. Make cylc clean an option of cylc install.
    • I.E. cylc install foo --clean.
    • Don't support any cylc clean options via cylc install for now, default behaviour only.
  2. Make the "Review" category options of cylc play
    • Initially cylc play --tui (easy).
    • Near future add cylc play --gui (requires UIS work).
    • Don't worry about cylc review.
    • These options are incompatible with --no-detach.
  3. Kick rose macro down the line for now, it's tricker to solve.
    • rose macro has a fair few options, the command won't necessarily do what you want it to do without specifying some of these.
    • We could allow defaults to be configured in the rose-suite.conf.
    • After that we can consider making rose macro an option of cylc validate via a cylc-rose interface.

Implement the following commands:

  1. vip
    • validate-install-play
    • Works with source directories in the same way as cylc install.
  2. vrp
    • validate-reinstall-[play/reload]
    • Consider calling vra (a for apply) or something like that to convey the play/reload abstraction.
    • Works with run IDs the same way as cylc play.
    • Uses play if the workflow is stopped, uses reload if the workflow is running.
    • Support options of cylc play but not cylc reload (which doesn't have any options anyway).
  3. sp (implement in cylc-rose)
    • stem-play
    • Works with source directories in the same way as rose stem

Intermingle the CLI options e.g:

# -s goes to validate & play
# --pause goes to play
cylc vip -s ANSWER=42 --pause

Attempt to do this in a way which minimises duplication, drop any incompatible arguments. Dump all of the options into a single object and provide it to each of the commands called.

Examples:

# validate "foo" from "[install]source dirs" then install and play it
$ cylc vip foo

# validate the source of bar/run1 (using CLI options scraped from run dir)
# then reinstall and play/reload it depending on workflow state
cylc vrp "bar/run1"

# install and run the rose stem workflow defined in $PWD
$ cylc sp

$ cylc vip foo --gui  # closest approximation of rose suite-run
$ rose sp --gui  # closet approximation of rose stem (@ Rose 2019), possibly `cylc sp`, WIP...

@hjoliver
Copy link
Member

hjoliver commented May 12, 2022

I'm not convinced the benefits are worth the effort and - by some measures at least - extra complexity (even for users, who will likely have to know the base commands and the "alias" commands, and the command option constraints for different aliases).

git pull makes good sense (*pull changes into this branch" == "fetch changes, and merge them into this branch")

But if we can't think of equally intuitive alias names, is there much benefit? It's not that hard to type cylc validate . && cylc install && cylc play <name> - and add your own options to each command as needed.

What about just giving users the power to create their own aliases, like git aliases? (If good old shell aliases are insufficient).

@oliver-sanders
Copy link
Member Author

What about just giving users the power to create their own aliases

Doesn't really work because of the need to distribute the CLI options e.g. consider:

$ cylc validate -s ANSWER=42 --opt-conf-key=foo; cylc install --opt-conf-key=foo; cylc play -s ANSWER=42

I'm not convinced the benefits are worth the effort

To give a vauge idea of required effort:

  • cylc play --tui & cylc install --clean
    • Should be quick and easy.
  • cylc vip
    • Needs a new cylc.flow.scripts command.
    • Would import validate/install/play logic.
    • Needs some lines of code to distribute the CLI options appropriately.
  • cylc vrp

@dpmatthews
Copy link
Contributor

Validate before reinstall is good practise but currently not easy to do (you need to know what install/play options were used) - cylc vrp will address this.

Another advantage of providing these commands is that we can also provide access to them in the UI - users are going to want this.

@hjoliver
Copy link
Member

OK, fair points - agreed.

@wxtim
Copy link
Member

wxtim commented May 17, 2022

To find out for sure we will have to poll users.

Or just do some options, and listen

+ui (tui likely to be a popular choice w/ rose stem)

I don't think that we should worry too much about UIs because 2/3 of them are long term open.

The letters I've used are illustrative, we could go with them, however, commands such as vro aren't the most intuitive but what's the alternative?!

Might it be possible to allow both - I have it in my head that it'd be nice if Cylc Reminded people what the contraction was every time they use it. (In my head [NOTE] Something is in green)

$ cylc vip
[NOTE] Cylc vip is short for cylc validate-install-play
....

$ cylc validate-install-play
[NOTE] Cylc validate-install-play can be contracted to cylc vip
...

@dpmatthews
Copy link
Contributor

@hjoliver Are you happy for the Question label to be removed now?

@hjoliver hjoliver removed the question Flag this as a question for the next Cylc project meeting. label May 17, 2022
@hjoliver
Copy link
Member

To add to this, today I have been using the following pattern which also incorporates cylc clean:

(I do this too)

@hjoliver
Copy link
Member

I have it in my head that it'd be nice if Cylc Reminded people what the contraction was every time they use it.

@wxtim I quite like this idea. I know people who use aliases so often (normally their own shell aliases) that they can't communicate with others anymore, in terms of the real command names 🙄

@oliver-sanders
Copy link
Member Author

To start with we could do something like this (where the <b /> tag means bold in ansimarkup):

cylc validate-install-play    
                     
<b>$ cylc validate</b>    
WARNING: ...       
INFO: ...        
valid for ...      
                 
<b>$ cylc install</b>
...              
                 
<b>$ cylc play</b>
...        

Long term we could do something more Tui style (e.g. #4778 (comment)) and make it dynamic (i.e. collapse the previous section of output when moving onto the next, excluding warnings of course) and avoid duplicating the warning output from parsing the config twice (i.e. we need config warnings for validate OR play not both). But for the short term something quick and simple.

@wxtim
Copy link
Member

wxtim commented Jul 4, 2022

Review of possible clashes of CLI args

ic (clean, install)

@oliver-sanders suggests doing this as cylc install --clean.

Don't support any cylc clean options via cylc install for now, default behaviour only.

Which would be easy.

I'm not keen on confusing clean and install like this, and I'd rather have a new script to make it clear that it is gathering up atomic CLI commands.

Alternative IC (if we want more sophisticated handling)

  • args: cylc clean takes a workflow id. If we don't want this to succeed if there is nothing to clean we can use the source link in the installed dir to get the source. Else we would have to explicitly give a re-install source: Do we want this to work if the workflow hasn't been installed before (clean just does nowt)? I thought why not, but actually, if someone is trying to do this and not cleaning something they might be doing something wrong.
  • --rm, -y, --timeout: only in cylc install - I think that this is safe and reasonable.
  • --local, --remote: only in cylc install - Would it be odd to do this? Are there cases where the stuff the user wants to clean and replace will only be stuff locally installed. Propose disable these.
  • --comms-timeout: only in Cylc install, although a wee bit confusing with cleans --timeout. IMO any combined CLI should spell out clearly that one of these is going to install and one to clean.
  • Shared commands - fine - these control things like logging level &c.
  • Other non-shared items in cylc install. Don't see why we can't let users access these.

@wxtim
Copy link
Member

wxtim commented Jul 4, 2022

VIP (validate-install-play)

  • Should validate's arg be WORKFLOW|PATH as described or WORKFLOW|PATH|SOURCE?
  • cylc validate --output-filename probably isn't very useful to VIP and should probably be forced unset.
  • cylc install -n (workflow name) clashed with cylc play -n (no-detach) - proposal -n == --workflow-name because users are more likely to want that. Consider logging a warning bc it's still ambiguous.
  • Possibe DANGER: cylc-rose options for cylc install (-S -O -D) could be over-run at play by --set and --set-file, This can happen anyway, but there's potential for it to be obscured by the VIP command, or even, confusingly, fail validating after install which it passed before.

@wxtim
Copy link
Member

wxtim commented Jul 4, 2022

VRP/VRA (validate-reinstall-play/reload(apply?))

  • As with VIP --output-filename can probably be ditched for the validate step.
  • No other clashes.

@wxtim
Copy link
Member

wxtim commented Jul 4, 2022

SP

Same issues as VIP.

@wxtim
Copy link
Member

wxtim commented Aug 24, 2022

Is cylc install --clean just as well to think of as cylc reinstall --clean?

@wxtim wxtim mentioned this issue Sep 7, 2022
7 tasks
@oliver-sanders
Copy link
Member Author

Pushing the rose stem aspect of this issue to - cylc/cylc-rose#205

@oliver-sanders
Copy link
Member Author

Pushing the tui / gui side to - #5300

@oliver-sanders
Copy link
Member Author

Finally punting the rose macro -V suggestion to - #5301

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

Successfully merging a pull request may close this issue.

4 participants