epugh commented on code in PR #4320:
URL: https://github.com/apache/solr/pull/4320#discussion_r3328839989
##########
solr/core/src/java/org/apache/solr/cli/ExportTool.java:
##########
@@ -281,16 +281,16 @@ public void streamDocListInfo(long numFound, long start,
Float maxScore) {}
@Override
public void runImpl(CommandLine cli) throws Exception {
String url;
- if (cli.hasOption(CommonCLIOptions.SOLR_URL_OPTION)) {
+ if (CLIUtils.hasConnectionOption(cli)) {
if (!cli.hasOption(COLLECTION_NAME_OPTION)) {
throw new IllegalArgumentException(
- "Must specify -c / --name parameter with --solr-url to post
documents.");
+ "Must specify -c / --name parameter with a connection target to
export documents.");
}
url = CLIUtils.normalizeSolrUrl(cli) + "/solr/" +
cli.getOptionValue(COLLECTION_NAME_OPTION);
} else {
- // think about support --zk-host someday.
- throw new IllegalArgumentException("Must specify --solr-url.");
+ throw new IllegalArgumentException(
+ "Must specify a connection target via -s/--solr-connection,
--solr-url, or --zk-host.");
Review Comment:
okay, I dug in a bit, and this is a weakness in our current CLI, and it's in
a couple of places. We do our own checking instead of relying on a pattern
like:
```
OptionGroup connectionOptions = getConnectionOptions();
connectionOptions.setRequired(true);
```
Having said that, we have a new Picocli based CLI coming, that does things
better. I'm inclined to put a note about this on that work effort, and not
expand/modify this PR.
Good eyes David!
--
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]
