Skip to content

CLI: Fix SOLR_CONNECTION fallback and other small improvements - #5054

Merged
janhoy merged 3 commits into
apache:mainfrom
janhoy:picocli-cli-small-fixes
Oct 7, 2026
Merged

janhoy merged 3 commits into
apache:mainfrom
janhoy:picocli-cli-small-fixes

Conversation

@janhoy

@janhoy janhoy commented Oct 7, 2026

Copy link
Copy Markdown
Contributor
  • SOLR_CONNECTION fallback was dead on both parser paths. CliDefaultValueProvider and CLIUtils.resolveSolrConnectionFromCli read the solr-connection property, but EnvUtils maps SOLR_CONNECTION to solr.connection and its camelCase fallback can't bridge a hyphen, so the documented "unnecessary if SOLR_CONNECTION is defined in solr.in.sh" never worked. Now reads solr.connection. Covered by the new CliDefaultValueProviderTest (picocli) and two tests in CLIUtilsTest (commons-cli).
  • bin/solr start --help (picocli) still advertised --prompt-inputs, which was renamed to --script-inputs in SOLR-18468; bin/solr and RunExampleTool already use the new name.
  • bin/solr create --help example used -s 2 for shards; -s is --solr-connection, shards is -sh.

Ref-guide CLI pages regenerated (solr-start.adoc, solr-create.adoc). No behaviour change on the commons-cli path other than the SOLR_CONNECTION fallback now taking effect.

…ate example typo

The solr-connection property key read by CliDefaultValueProvider and
CLIUtils never matched the solr.connection sysprop that SOLR_CONNECTION
maps to, so the documented fallback was dead on both parser paths.
StartCommand still advertised --prompt-inputs after the rename to
--script-inputs, and the create --help example used -s (solr-connection)
where -sh (shards) was meant.
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests cat:cli labels Oct 7, 2026
@janhoy
janhoy requested a review from epugh October 7, 2026 09:06
@janhoy janhoy changed the title PicoCLI: Fix SOLR_CONNECTION fallback and other small improvements CLI: Fix SOLR_CONNECTION fallback and other small improvements Oct 7, 2026
Comment on lines +39 to +43
} finally {
System.clearProperty("solr.connection");
System.clearProperty("zkHost");
System.clearProperty("solr.url");
}

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.

Our tests clean up system properties. Do you not know this? LLMs didn't get the memo...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yea. Note that SolrTestCase only cleans up sysprops at the end of the test class, so individual test methods need to handle props individually. But I suppose methods within a test class always runs sequentially so they won't interfere, and since testNoDefaultWhenPropertiesUnset clears props itself, there is no need for an explicit cleanup here. Fixed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added a line to AGENTS...

@janhoy
janhoy merged commit 5e044a3 into apache:main Oct 7, 2026
6 checks passed
@janhoy janhoy added this to the 10.x milestone Oct 7, 2026
janhoy added a commit to jaykay12/solr that referenced this pull request Oct 7, 2026
Main renamed the key from 'solr-connection' to 'solr.connection' in apache#5054;
EnvUtils maps SOLR_FOO -> solr.foo, so the hyphenated key was never set.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cat:cli documentation Improvements or additions to documentation tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants