janhoy opened a new pull request, #5063: URL: https://github.com/apache/solr/pull/5063
# Description The now-complete picocli port of `bin/solr` ended up with three parallel declarations of `--solr-connection`, `--zk-host` and `--solr-url`: - `ConnectionOptions`, an exclusive `@ArgGroup` used by most tools - `ZkConnectionOptions`, a `@Mixin` for the zk-family tools that also re-declared `--credentials` - a private clone inside `StatusTool` Because picocli applies no default-value-provider defaults to an unmatched `@ArgGroup`, the `SOLR_CONNECTION` / `ZK_HOST` / `SOLR_URL` fallback never reached the ArgGroup-based tools, so every one of them hand-rolled its own URL resolution with its own environment fallback and warning text. The zk-family tools accepted `--zk-host a --solr-url b` while the others rejected it, and `CliDefaultValueProvider` keyed on `paramLabel` (the Java field name), so setting a `paramLabel` silently disabled the environment fallback. # Solution - `ConnectionOptions` is now a single public `@Mixin` with `resolveSolrUrl`, `resolveZkHost`, `namedConnection`, `resolveSolrConnection` and `hasExplicitConnection` built in. An option passed on the command line wins over environment-populated fields, and passing more than one of the three explicitly is a usage error in every tool. - `ZkConnectionOptions` and `StatusTool.TargetOptions` are removed. Every tool mixes in `ConnectionOptions` and `CredentialsOptions` and drops its private resolver. `CollectionNameOptions` is used for `-c/--name` wherever it is required. - `CliDefaultValueProvider` is keyed by option name instead of `paramLabel`. Only the mixin's `--solr-url` takes the `SOLR_URL` default, since `ApiTool`'s own `--solr-url` names a full endpoint URL. - `ToolBase` lets a `ParameterException` raised while running reach picocli, so it is reported like any other invalid input (exit code 2 plus usage). - `SolrCLI.rootCommandLine` builds the fully configured root command for `main` and for tests that drive a tool through the real root. - CLI reference pages regenerated. The commons-cli path is unchanged. # Tests - `ConnectionOptionsTest` rewritten as parsing-level tests: precedence, environment-vs-explicit, mutual exclusion, non-ZK `--zk-host` rejection. - `CliDefaultValueProviderTest` rewritten for option-name keying, including the `ApiTool` opt-out. - All existing `*PicocliTest` twins pass (`org.apache.solr.cli.*`, 290 tests), as does `integrationTestsPicocli` (BATS) and `gradlew check -x test`. - Manual checks on the built distribution: `ZK_HOST`, `SOLR_URL` and `SOLR_CONNECTION` fallbacks now reach `zk ls`, `create` and `healthcheck`; `--zk-host` plus `--solr-url` exits 2; an environment `ZK_HOST` together with an explicit `--solr-url` succeeds. 🤖 Coded entirely with Claude Code -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
