Skip to content

Enable picocli for a few tools#3247

Merged
janhoy merged 1 commit intoapache:jira/SOLR-17697-picoclifrom
janhoy:feature/picocli
Apr 5, 2026
Merged

Enable picocli for a few tools#3247
janhoy merged 1 commit intoapache:jira/SOLR-17697-picoclifrom
janhoy:feature/picocli

Conversation

@janhoy
Copy link
Copy Markdown
Contributor

@janhoy janhoy commented Mar 8, 2025

PR that will merge into the Picocli feature branch visualized by #3254. This PR adds:

  • Parses bin/solr with picocli by default but also retain commons-cli for comparison. Switch using SOLR_PICOCLI=true
  • Implements VersionTool, StatusTool, StartTool, StopTool as sub commands
  • New ZkTool sub command, which in turn has ZkLsTool as a sub command
  • Demonstrate use of custom defaultValueProvider to let --zk-host option look for a value in environment

This 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:

SOLR_PICOCLI=true bin/solr
Missing required subcommand
Usage: solr [-hV] [COMMAND]
Global options:
  -h, --help      Show this help message and exit.
  -V, --version   Print version information and exit.

Commands:
  start    Starts Solr in standalone or SolrCloud mode.
  stop     Stops Solr.
  status   Get the status of a Solr server.
  version  Prints the Solr version.
  zk       Sub commands for working with ZooKeeper.

SolrCloud example (embedded Zookeeper):

  ./solr start

Get help for a command by running 'solr COMMAND --help'.

For more help on how to use Solr, head to https://solr.apache.org/

Copy link
Copy Markdown
Contributor

@epugh epugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

@janhoy
Copy link
Copy Markdown
Contributor Author

janhoy commented Mar 10, 2025

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 MYENV=123 bin/solr foo --bar baz it should result in the same tool behavior as before. It could be a 10.0 only or also a 9.x feature depending on whether we feel that tool usage output is part of our back-compat guarantees. Perhaps 10.0 is safest, although backporting CLI bugfixes will be harder if 9x is still on commons-cli.

I'd be interested in chipping away on this in a central collaborative feature branch over the course of a several weeks..

@epugh
Copy link
Copy Markdown
Contributor

epugh commented Mar 11, 2025

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 MYENV=123 bin/solr foo --bar baz it should result in the same tool behavior as before. It could be a 10.0 only or also a 9.x feature depending on whether we feel that tool usage output is part of our back-compat guarantees. Perhaps 10.0 is safest, although backporting CLI bugfixes will be harder if 9x is still on commons-cli.

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?

@janhoy janhoy changed the base branch from main to jira/SOLR-17697-picocli March 11, 2025 11:44
@janhoy janhoy changed the title POC: Test picocli Enable picocli for a few tools Mar 11, 2025
@janhoy
Copy link
Copy Markdown
Contributor Author

janhoy commented Mar 11, 2025

I made a JIRA and a feature branch with only the gradle stuff.
Re-purposed this PR to add the first few tools as a POC.
More PRs should follow targeting that branch.

To see status of the overall feature branch, I made a draft PR #3254

@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Mar 12, 2025
@janhoy
Copy link
Copy Markdown
Contributor Author

janhoy commented Aug 4, 2025

Guess I intend to continue on this, re-opening

@janhoy janhoy reopened this Aug 4, 2025
@github-actions github-actions bot removed closed-stale Closed after being stale for 60 days stale PR not updated in 60 days labels Aug 5, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 4, 2025

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!

@github-actions github-actions bot added the stale PR not updated in 60 days label Oct 4, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 3, 2025

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants