Enable picocli for a few tools#3247
Conversation
epugh
left a comment
There was a problem hiding this comment.
This looks super interesting! I briefly looked at Picocli based on @janhoy input back when we started revamping the cli in 2024, but it felt a step too far, so we focused on just getting all our args reworked.
Now, I think that picocli brings a lot of great features. One that I am quite intrigued by is generating docs for our CLI in AsciiDoc: https://picocli.info/#_generate_man_page_documentation, I took a stab at generating docs from commons-cli, but it was too hard. Plus the support for sub commands looks interesting... I think this is a really good direction...
|
|
||
| @Override | ||
| public int callTool() throws Exception { | ||
| if (zkHost == null) { |
There was a problem hiding this comment.
i wonder if we should have a more generic abstraction that isn't tied to "zkHost" for detecting if we are in solr cloud mode? Maybe an explicit method, because I feel like we are slowly hiding zkHost more and more, and the client may just be a solrClient configured with a solrUrl. <-- this isn't specific to your picocli change I know!
|
Eric, you have really lifted the CLI in the last versions, none of which is wasted if/when moving to picocli! Also, we have pretty good test coverage in bats tests, which is a great way to validate parity. It's not a goal to retain the exact same help/usage output. Let the tool generate based on best practices. However, given a CLI invocation I'd be interested in chipping away on this in a central collaborative feature branch over the course of a several weeks.. |
I'd love to work with you on this... I lean towards this being a 10x feature, just so we don't have to backport, and we have some time to make it "perfect". I think CLI bugfixes on 9x will just being and end on 9x branch in this case. So what is next? A branch in the main asf repo? Or do we just push to this one? |
|
Guess I intend to continue on this, re-opening |
|
This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the dev@solr.apache.org mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution! |
|
This PR is now closed due to 60 days of inactivity after being marked as stale. Re-opening this PR is still possible, in which case it will be marked as active again. |
- start - stop (just help) - status - version - zk ls Add callTool() stub to all other tools
PR that will merge into the Picocli feature branch visualized by #3254. This PR adds:
bin/solrwith picocli by default but also retain commons-cli for comparison. Switch usingSOLR_PICOCLI=trueVersionTool,StatusTool,StartTool,StopToolas sub commandsZkToolsub command, which in turn hasZkLsToolas a sub commanddefaultValueProviderto let--zk-hostoption look for a value in environmentThis is the first of many PRs to go against the feature branch to try to build full picocli coverage for the CLI tools.
Sample output for
bin/solr -h: