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

Introduce PicoCLI for parsing CLI arguments #875

Merged
merged 1 commit into from
Sep 3, 2019
Merged

Introduce PicoCLI for parsing CLI arguments #875

merged 1 commit into from
Sep 3, 2019

Conversation

prd-fox
Copy link
Contributor

@prd-fox prd-fox commented Sep 2, 2019

Note this is only fully replaced for the Admin CLI adapter to start with.

The Enclave and Default CLI adapter can come after we are happy with it.
Until this time, there is some duplication of functionality, e.g. PID file parsing.


PicoCLI allows for a top level command, and then nesting commands under it.

For example, tessera admin -addpeer <url> -configfile <path> has 3 commands:

tessera - the main "top level command", given by DefaultCliAdapter
admin - the first subcommand, given by AdminCommand and included in the top level command.
-addpeer - the next subcommand, put under AddPeerAction. This one is a bit annoying, since we decided to go with combining the action and parameter in the same argument. It would have been more idiomatic to use ... addpeer -url ..., but has been kept for backwards-compatibility.
The ability to create a PID file is common functionality. It has been included in the addpeer command directly, instead of being at a common point higher up in the tree (i.e. having it apply to every subcommand automatically), because I haven't figured out yet if thats possible.
New commands can remain entirely separate, and only need to worry about themselves. A reference is added the parent command, and the library takes care about invoking it.

Below is an example of how the subcommands are laid out and routed, including some that don't exist for explanation purposes.

tessera (enclave or TM)
|
|
|----- admin
|        |
|        |----- -addpeer
|        |
|        |----- migrate
|        |
|        |----- deleteForKey
|
|----- -keygen
|
etc

Notable user differences:

help messages have more information, but are limited to the (sub)command they are displaying, instead of showing everything at once. (like git help shows all top level commands, git help add shows only the add subcommand help)

command line args are ordered to the command they are for, instead of being in any order. e.g.
java -jar tessera.jar -configfile <path> admin -addpeer <url>
becomes
java -jar tessera.jar admin -addpeer <url> -configfile <path>
so whilst this may be considered breaking, it is small and actually enforces more standard usage.

Part of #874

Dynamically load the CliAdapters and pass them into PicoCLI
@codecov-io
Copy link

codecov-io commented Sep 2, 2019

Codecov Report

Merging #875 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #875      +/-   ##
============================================
+ Coverage     99.44%   99.44%   +<.01%     
- Complexity     2071     2097      +26     
============================================
  Files           312      317       +5     
  Lines          6075     6109      +34     
  Branches        308      310       +2     
============================================
+ Hits           6041     6075      +34     
  Misses            5        5              
  Partials         29       29
Impacted Files Coverage Δ Complexity Δ
.../main/java/com/quorum/tessera/cli/CliDelegate.java 100% <100%> (ø) 11 <7> (+2) ⬆️
...quorum/tessera/cli/parsers/ConfigurationMixin.java 100% <100%> (ø) 3 <3> (?)
...m/quorum/tessera/config/cli/DefaultCliAdapter.java 99.36% <100%> (+0.01%) 22 <2> (+2) ⬆️
...a/com/quorum/tessera/cli/CLIExceptionCapturer.java 100% <100%> (ø) 3 <3> (?)
.../com/quorum/tessera/admin/cli/AdminCliAdapter.java 100% <100%> (ø) 4 <3> (-6) ⬇️
...om/quorum/tessera/cli/parsers/ConfigConverter.java 100% <100%> (ø) 3 <3> (?)
...a/com/quorum/tessera/cli/parsers/PidFileMixin.java 100% <100%> (ø) 6 <6> (?)
.../tessera/admin/cli/subcommands/AddPeerCommand.java 100% <100%> (ø) 11 <11> (?)
...orum/tessera/enclave/server/EnclaveCliAdapter.java 100% <100%> (ø) 8 <3> (+2) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b313e17...5e70d7b. Read the comment docs.

@Krish1979 Krish1979 merged commit e65b052 into Consensys:master Sep 3, 2019
@prd-fox prd-fox added 0.11 feature User features labels Sep 3, 2019
@prd-fox prd-fox deleted the picocli branch September 3, 2019 12:54
@Krish1979 Krish1979 added 0.10.1 and removed 0.11 labels Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.10.1 feature User features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants