fortify / fcli

fcli is a command-line utility for interacting with various Fortify products
https://fortify.github.io/fcli/
Other
31 stars 17 forks source link

`fcli * wait-for`: Reconsider option design #350

Closed rsenden closed 1 year ago

rsenden commented 1 year ago

Various optional options on wait-for commands accept one or more states, like --until, --while, ... At the moment, multiple states are separated by the pipe separator, i.e. --until PROCESS_COMPLETE|REQUIRE_AUTH, whereas most/all other commands use comma's to separate repeatable option values.

Also, these --until* and --while* options are not real repeatable options, for example you can't specify --until PROCESS_COMPLETE --until REQURE_AUTH as an alternative for --until PROCESS_COMPLETE|REQUIRE_AUTH. Again, this is inconsistent with other commands. It also means that auto-completion is not available for these options, whereas it would be very helpful for users to have auto-completion show the supported states.

The main reason for using | as the separator is that this represents an 'OR' operator, with the following reading as 'wait for entity1 and entity2 until all given entities reach state1 or state2:

fcli ... wait-for <entity1> <entity2> --until-all <state1>|<state2>

When using , as the separator, it becomes less clear as to whether --until-all refers to 'all entities' or 'all states', especially if only a single entity is specified. For example, the following might be read as 'wait for entity until it has both state1 and state2, which obviously can't happen (entity can have only single state) but may still cause confusion:

fcli ... wait-for <entity> --until-all <state1>,<state2>

So, we should either accept the fact that the wait-for commands don't offer auto-completion for supported states and have a different structure than other commands, sticking to the current | separator, or we need to redesign the option structure.

Some alternative structures that have come to mind so far:

1:
fcli ssc artifact wait-for 12 13 --wait until-all-match --states REQUIRE_AUTH,PROCESS_COMPLETE
fcli ssc artifact wait-for 12 13 --wait while-any-match --states SCHED_PROCESSING,PROCESSING

2:
fcli ssc artifact wait-for 12 13 --until-all-match --states REQUIRE_AUTH,PROCESS_COMPLETE
fcli ssc artifact wait-for 12 13 --while-any-match --states SCHED_PROCESSING,PROCESSING

3:
fcli ssc artifact wait-for 12 13 --wait until --match all-artifacts --states REQUIRE_AUTH,PROCESS_COMPLETE
fcli ssc artifact wait-for 12 13 --wait while --match any-artifact --states SCHED_PROCESSING,PROCESSING

4:
fcli ssc artifact wait-for 12 13 --until --match all-artifacts --states REQUIRE_AUTH,PROCESS_COMPLETE
fcli ssc artifact wait-for 12 13 --while --match any-artifact --states SCHED_PROCESSING,PROCESSING

5:
fcli ssc artifact wait-for 12 13 --until --match-all-artifacts --states REQUIRE_AUTH,PROCESS_COMPLETE
fcli ssc artifact wait-for 12 13 --while --match-any-artifact --states SCHED_PROCESSING,PROCESSING

Of these, option 3 is probably most consistent with other commands and the most explicit as to what each option/value means, at the cost of being the longest. All other options use exclusive boolean options (like --until and --while being mutually exclusive boolean options):

Note that any of these options should be optional. For example, for the fcli ssc artifact wait-for command, the only required input would be one or more artifact id's; if no options are specified, this command would wait until all given artifacts reach PROCESS_COMPLETE status.

psmf22 commented 1 year ago

I´d agree with the assessment that having the pipe operator makes it more clear that it is an OR. In terms of command style i would prefer option #2 for brevity. In case you go for option #3 i would propose to make the --match keywords not entity specifc (so "all" rather than "all-artifacts" or "any" instead of "any-artifacts", or to avoid confusion between states and entities you could use all-entities)

rsenden commented 1 year ago

@psmf22 Given that you agree that the pipe operator makes it more clear that it's an OR, do you think we should just keep the current approach, i.e.:

fcli ssc artifact wait-for 12 13 --until-all REQUIRE_AUTH|PROCESS_COMPLETE
fcli ssc artifact wait-for 12 13 --while-any SCHED_PROCESSING|PROCESSING

It's shorter than the other options, and given the pipe/OR symbol, it's clear that any or all refers to the entities rather than states. On the other hand, the pipe symbol is less visible between uppercase letters compared to comma's, it's inconsistent with other fcli options taking multiple values, and could require the value to be quoted to avoid the shell interpreting this as a pipe symbol.

Of course, any other suggestions to have the best of both worlds are welcome.

rsenden commented 1 year ago

Adding options 6 & 7 as additional ideas:

Current: 
fcli ssc artifact wait-for 12 13 --until-all REQUIRE_AUTH|PROCESS_COMPLETE
fcli ssc artifact wait-for 12 13 --while-any SCHED_PROCESSING|PROCESSING

1:
fcli ssc artifact wait-for 12 13 --wait until-all-match --states REQUIRE_AUTH,PROCESS_COMPLETE
fcli ssc artifact wait-for 12 13 --wait while-any-match --states SCHED_PROCESSING,PROCESSING

2:
fcli ssc artifact wait-for 12 13 --until-all-match --states REQUIRE_AUTH,PROCESS_COMPLETE
fcli ssc artifact wait-for 12 13 --while-any-match --states SCHED_PROCESSING,PROCESSING

3:
fcli ssc artifact wait-for 12 13 --wait until --match all-artifacts --states REQUIRE_AUTH,PROCESS_COMPLETE
fcli ssc artifact wait-for 12 13 --wait while --match any-artifact --states SCHED_PROCESSING,PROCESSING

4:
fcli ssc artifact wait-for 12 13 --until --match all-artifacts --states REQUIRE_AUTH,PROCESS_COMPLETE
fcli ssc artifact wait-for 12 13 --while --match any-artifact --states SCHED_PROCESSING,PROCESSING

5:
fcli ssc artifact wait-for 12 13 --until --match-all-artifacts --states REQUIRE_AUTH,PROCESS_COMPLETE
fcli ssc artifact wait-for 12 13 --while --match-any-artifact --states SCHED_PROCESSING,PROCESSING

6:
fcli ssc artifact wait-for 12 13 --until all-artifacts --match-any-state REQUIRE_AUTH,PROCESS_COMPLETE
fcli ssc artifact wait-for 12 13 --while any-artifact --match-any-state SCHED_PROCESSING,PROCESSING

7:
fcli ssc artifact wait-for 12 13 --until all --match-any-state REQUIRE_AUTH,PROCESS_COMPLETE
fcli ssc artifact wait-for 12 13 --while any --match-any-state SCHED_PROCESSING,PROCESSING

With option 7, given that we now explicitly have --match-any-state, it's probably more clear that all or any refers to the artifacts, not states, so we can potentially use this shorter and more generic form instead of all-<entity> or any-<entity> as in option 6.