reanahub / reana-client

REANA command-line client
http://reana-client.readthedocs.io/
MIT License
10 stars 44 forks source link

cli: implement new share-add command #680

Open tiborsimko opened 8 months ago

tiborsimko commented 8 months ago

For the workflow sharing sprint, design and implement an initial share command allowing to share workflow runs with colleagues.

For example, Alice shares her myanalysis run 42 with Bob:

$ reana-client share -w myanalysis.42 --share-with bob@example.org
myanalysis.42 is now read-only shared with bob@example.org

For example, Alice enquires with whom she shared her myanalysis run 42:

$ reana-client share -w myanalysis.42
myanalysis.42 has been shared with:
- bob@example.org
- cecile@example.org

The CLI could have JSON output for easy piping into user scripts (as the list, status etc commands):

$ reana-cilent share -w myanalysis.42 --json | jq
[ 
    "bob@example.org",
    "cecile@example.org"
]

P.S. This issue depends on https://github.com/reanahub/reana-db/issues/206

DaanRosendal commented 8 months ago

It might be more intuitive to use a dedicated command for enquiring who a workflow has been shared with:

reana-client shared-with -w myanalysis.42

What do you think?

tiborsimko commented 8 months ago

I don't like that grammatically the shared-with command would differ from the share and unshare prefix-wise. They are closely connected but this connection may be lost in the grammar.

1) What about using a common prefix instead as for secrets-*? This would give for example:

$ reana-client share-add -w myanalysis.42 --user bob@cern.ch --user cecile@cern.ch --group lhcb-general@cern.ch
$ reana-client share-status -w myanalysis.42
$ reana-client share-remove -w myanalysis.42 --user bob@cern.ch --user cecile@cern.ch --group lhcb-general@cern.ch

Here, the CLI option would be mandatory, which may not be the usual behaviour of CLI switches.

2) We could also think of splitting the command into more separate subcommands which would allow us to go for mandatory arguments:

$ reana-client share-add-user -w myanalysis.42 bob@cern.ch cecile@cern.ch
$ reana-client share-add-group -w myanalysis.42 lhcb-general@cern.ch
$ reana-client share-status -w myanalysis.42
$ reana-client share-remove-user -w myanalysis.42 bob@cern.ch cecile@cern.ch
$ reana-client share-remove-group -w myanalysis.42 lhcb-general@cern.ch

3) Note that in the near future we may have several more optional arguments to express the optional sharing validity time interval and the optional invitation message, for example:

$ reana-client share-add -w myanalysis.42 \
      --user bob@cern.ch \
      --expires 2024-12-31 \
      --message "hi bob! check my new plots" \
      --notify

This should also be taken into account for a possible full CLI argument and option design.

4) Note also another possible CLI argument design that would be "action-based", such as:

$ reana-client share -w myanalysis.42 --add-user bob@cern.ch --remove-user cecile@cern.ch
$ reana-client share -w myanalysis.42 --report

5) The above alternatives are to be compared to the original proposal:

$ reana-client share -w myanalysis.42 --user bob@cern.ch --user cecile@cern.ch
$ reana-client share -w myanalysis.42 --status
$ reana-client unshare -w myanalysis.42 --user bob@cern.ch --user cecile@cern.ch
$ reana-client unshare -w myanalysis.42 --status
DaanRosendal commented 8 months ago

I prefer option 1 for several reasons:

  1. Clear separation of share and unshare Commands: It maintains a clear distinction between the "share" and "unshare" commands, ensuring that users can easily discern their intended actions. In combined commands, such as in option 4, it could become confusing to determine whether a message is meant for those being added, removed, or both.
  2. Consistency with existing conventions: This approach aligns with the existing "secret-*" command structure, making it consistent.
  3. Enhanced user-friendliness: It provides clear visibility of available options, preventing them from being buried within flags or subcommands, as seen in other options. Users might appreciate this straightforward approach.
  4. Preventing overloaded help page: While option 2, with its separate subcommands, offers a well-structured CLI, it has the drawback of potentially overloading the reana-client --help page with numerous commands. Option 1 strikes a balance between usability and a manageable command set, keeping the --help page clean and concise.

@mdonadoni @giuseppe-steduto Do you have any strong opinions here?

giuseppe-steduto commented 8 months ago

Personally, I like option 1 as well, and I share your opinion, especially because of the consistency with the secrets-* commands!

I just wanted to point out that maybe it can be worth considering splitting it in two different subcommands: reana-client share-add becomes reana-client share add, reana-client share remove, and so on. This is similar to what gh does for example with its subcommands (think gh pr create, gh pr checkout, and so on). Other tools, like heroku, use colons rather than spaces to separate commands from the subcommands (to help delineate the command between the arguments passed to the command), but since we mainly use flags, I wouldn't consider this slightly exotic approach. (Note that if we go for this option, maybe we should refactor accordingly the secrets management commands / the quota-show command as well.)

Interesting and relevant read that inspired this comment: https://clig.dev/

mdonadoni commented 8 months ago

I also think option 1 is the best one, as it is consistent with the rest of reana-client's commands.

Regarding share-add vs share add, I do not have a strong preference about this, but I will have a look at clig.dev (thanks @giuseppe-steduto !). In any case, if we want to migrate to the "dash-less" version, it should not be difficult to keep both versions (e.g. both secrets-add and secrets add) for backwards compatibility.

tiborsimko commented 8 months ago

it should not be difficult to keep both versions (e.g. both secrets-add and secrets add) for backwards compatibility.

... but if you think of reana-dev --help, and of kubectl exec -i -t deployment/reana-server -- flask reana-admin --help, then we have quite a few places elsewhere that would be necessary to port as well (and beware of compatibility for some of these) for the overall consistency across the REANA project.

I'm not sure whether the benefits would outweigh the costs.

Using the flat command universe has also its positive sides. IMO, the number of the CLI commands we have is still relatively moderated. One positive is that the user gets an easy birds-eye-view about all the reana-client capabilities when doing --help, showing all the functionality in one go. With subcommands the user might need to "iterate inside" subcommands's help pages separately. Also, there may be certain cognitive advantage to follow reana-{dev,admin,client} <action> <argument> <flags> style, seeing clearly where the command action stops and where the arguments start. With subcommands the "action part" is variable in length, as it were.

So, whilst it may be good to go for subcommands for large CLI utilities such as kubectl, I'm not sure whether we are already there ourselves, and whether the porting would bring much benefits?