malliaridis commented on code in PR #2756:
URL: https://github.com/apache/solr/pull/2756#discussion_r1797711145
##########
solr/core/src/java/org/apache/solr/cli/PostLogsTool.java:
##########
@@ -66,24 +67,46 @@ public List<Option> getOptions() {
return List.of(
Option.builder("url")
.longOpt("solr-collection-url")
+ .deprecated(
+ DeprecatedAttributes.builder()
+ .setForRemoval(true)
+ .setSince("9.8")
+ .setDescription("Use --solr-url and -c / --name instead")
+ .get())
.hasArg()
.argName("ADDRESS")
- .required(true)
.desc("Address of the collection, example
http://localhost:8983/solr/collection1/.";)
.build(),
+ Option.builder("c")
+ .longOpt("name")
+ .hasArg()
+ .argName("NAME")`
+ .desc("Name of the collection.")
+ .build(),
Option.builder("rootdir")
.longOpt("rootdir")
.hasArg()
.argName("DIRECTORY")
.required(true)
.desc("All files found at or below the root directory will be
indexed.")
.build(),
+ SolrCLI.OPTION_SOLRURL,
SolrCLI.OPTION_CREDENTIALS);
}
@Override
public void runImpl(CommandLine cli) throws Exception {
- String url = cli.getOptionValue("url");
+ String url = null;
+ if (cli.hasOption("solr-url")) {
+ if (!cli.hasOption("name")) {
+ throw new IllegalArgumentException(
+ "Must specify -c / --name parameter with --solr-url to post
documents.");
+ }
+ url = SolrCLI.normalizeSolrUrl(cli) + "/solr/" +
cli.getOptionValue("name");
+
+ } else {
+ url = cli.getOptionValue("solr-collection-url");
Review Comment:
Same null-check applies here as well.
##########
solr/core/src/test/org/apache/solr/cli/PostToolTest.java:
##########
@@ -85,8 +85,10 @@ public void testBasicRun() throws Exception {
String[] args = {
"post",
- "--solr-update-url",
- cluster.getJettySolrRunner(0).getBaseUrl() + "/" + collection +
"/update",
+ "--solr-url",
+ cluster.getJettySolrRunner(0).getBaseUrl().toString(),
Review Comment:
`getBaseUrl()` extends v1 API path `/solr`, and the new logic does also add
`/solr/` if `--solr-url` is used. This results to a path like
`/solr/solr/collection/update`.
##########
solr/core/src/test/org/apache/solr/cloud/SolrCloudExampleTest.java:
##########
@@ -115,8 +115,10 @@ public void testLoadDocsIntoGettingStartedCollection()
throws Exception {
String[] argsForPost =
new String[] {
- "--solr-update-url",
- solrUrl + "/" + testCollectionName + "/update",
+ "--solr-url",
+ solrUrl,
Review Comment:
Same problem with path applies here as well, but this uses different logic.
##########
solr/core/src/java/org/apache/solr/cli/ExportTool.java:
##########
@@ -250,7 +263,17 @@ public void streamDocListInfo(long numFound, long start,
Float maxScore) {}
@Override
public void runImpl(CommandLine cli) throws Exception {
- String url = cli.getOptionValue("solr-collection-url");
+ String url = null;
+ if (cli.hasOption("solr-url")) {
+ if (!cli.hasOption("name")) {
+ throw new IllegalArgumentException(
+ "Must specify -c / --name parameter with --solr-url to post
documents.");
+ }
+ url = SolrCLI.normalizeSolrUrl(cli) + "/solr/" +
cli.getOptionValue("name");
+
+ } else {
+ url = cli.getOptionValue("solr-collection-url");
Review Comment:
Since `solr-collection-url` was required before, we should check for `null`
here and throw an error if it is also not provided.
##########
solr/core/src/test/org/apache/solr/cli/TestExportTool.java:
##########
@@ -243,8 +243,10 @@ public void testWithBasicAuth() throws Exception {
String[] args = {
"export",
- "-url",
- cluster.getJettySolrRunner(0).getBaseUrl() + "/" + COLLECTION_NAME,
+ "--solr-url",
+ cluster.getJettySolrRunner(0).getBaseUrl().toString(),
Review Comment:
Same issue with path here.
--
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]
