malliaridis commented on code in PR #2684:
URL: https://github.com/apache/solr/pull/2684#discussion_r1754439637
##########
solr/packaging/test/test_ssl.bats:
##########
@@ -526,12 +526,14 @@ teardown() {
# server1 will run on $SOLR_PORT and will use server1.keystore
export SOLR_SSL_KEY_STORE=$ssl_dir/server1.keystore.p12
export SOLR_SSL_TRUST_STORE=$ssl_dir/server1.keystore.p12
- solr start -c -a "-Dsolr.jetty.sslContext.reload.scanInterval=1
-DsocketTimeout=5000"
+ solr start -c --jvm-opts "-Dsolr.jetty.sslContext.reload.scanInterval=1
-DsocketTimeout=5000"
solr assert --started https://localhost:${SOLR_PORT} --timeout 5000
# server2 will run on $SOLR2_PORT and will use server2.keystore. Initially,
this is the same as server1.keystore
export SOLR_SSL_KEY_STORE=$ssl_dir/server2.keystore.p12
export SOLR_SSL_TRUST_STORE=$ssl_dir/server2.keystore.p12
+
+ # leaving -a instead of --jvm-opts for back compat testing.
solr start -c -z localhost:${ZK_PORT} -p ${SOLR2_PORT} -a
"-Dsolr.jetty.sslContext.reload.scanInterval=1 -DsocketTimeout=5000"
Review Comment:
This should fail based on my observations on a sample project. Can someone
confirm that this works correctly?
##########
solr/core/src/java/org/apache/solr/cli/RunExampleTool.java:
##########
@@ -140,54 +140,54 @@ public List<Option> getOptions() {
.build(),
Option.builder()
.longOpt("example-dir")
- .argName("DIR")
.hasArg()
+ .argName("DIR")
.required(false)
.desc(
"Path to the Solr example directory; if not provided,
${serverDir}/../example is expected to exist.")
.build(),
Option.builder()
.longOpt("url-scheme")
- .argName("SCHEME")
.hasArg()
+ .argName("SCHEME")
.required(false)
.desc("Solr URL scheme: http or https, defaults to http if not
specified.")
.build(),
Option.builder("p")
- .argName("PORT")
+ .longOpt("port")
.hasArg()
+ .argName("PORT")
.required(false)
.desc("Specify the port to start the Solr HTTP listener on;
default is 8983.")
- .longOpt("port")
.build(),
Option.builder()
- .argName("HOSTNAME")
+ .longOpt("host")
.hasArg()
+ .argName("HOSTNAME")
.required(false)
.desc("Specify the hostname for this Solr instance.")
- .longOpt("host")
.build(),
Option.builder("c")
+ .longOpt("cloud")
.required(false)
.desc(
"Start Solr in SolrCloud mode; if -z not supplied, an embedded
ZooKeeper instance is started on Solr port+1000, such as 9983 if Solr is bound
to 8983.")
- .longOpt("cloud")
.build(),
Option.builder("m")
- .argName("MEM")
+ .longOpt("memory")
.hasArg()
+ .argName("MEM")
.required(false)
.desc(
"Sets the min (-Xms) and max (-Xmx) heap size for the JVM,
such as: -m 4g results in: -Xms4g -Xmx4g; by default, this script sets the heap
size to 512m.")
- .longOpt("memory")
.build(),
- Option.builder("a")
- .argName("OPTS")
Review Comment:
By removing this option should the `DefaultParser` not throw an
`UnrecognizedOptionException` if the option is not present during parsing but
the user passes `-a` to the CLI?
This would break backwards compatibility as well if we plan to backport the
changes to 9X
##########
solr/bin/solr.cmd:
##########
@@ -426,7 +426,7 @@ IF "%1"=="-s" goto set_solr_url
IF "%1"=="--solr-url" goto set_solr_url
IF "%1"=="-solrUrl" goto set_solr_url
IF "%1"=="-a" goto set_addl_opts
-IF "%1"=="--additional-options" goto set_addl_opts
Review Comment:
Would we have to keep this for backwards compatibility in `branch_9x`?
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
