dsmiley commented on code in PR #3312:
URL: https://github.com/apache/solr/pull/3312#discussion_r2283261216
##########
solr/bin/solr.in.sh:
##########
@@ -248,14 +248,14 @@
#SOLR_SECURITY_MANAGER_ENABLED=true
# This variable provides you with the option to disable the Admin UI. If you
uncomment the variable below and
-# change the value to true. The option is configured as a system property as
defined in SOLR_START_OPTS in the start
+# change the value to false. The option is configured as a system property as
defined in SOLR_START_OPTS in the start
# scripts.
-# SOLR_ADMIN_UI_DISABLED=false
+# SOLR_UI_ENABLED=false
Review Comment:
flip default
##########
solr/bin/solr.in.cmd:
##########
@@ -224,12 +224,12 @@ REM set SOLR_SECURITY_MANAGER_ENABLED=true
REM This variable provides you with the option to disable the Admin UI. if you
uncomment the variable below and
REM change the value to true. The option is configured as a system property as
defined in SOLR_START_OPTS in the start
REM scripts.
-REM set SOLR_ADMIN_UI_DISABLED=false
+REM set SOLR_UI_ENABLED=false
Review Comment:
flip the default
##########
solr/solrj/src/resources/EnvToSyspropMappings.properties:
##########
@@ -5,29 +5,21 @@
# This way, env SOLR_FOO_BAR will also match property 'solr.foo.bar' without a
mapping in this file
# TODO: Deprecate non-standard sysprops and standardize on solr.foo.bar in
Solr 10
AWS_PROFILE=aws.profile
-DEFAULT_CONFDIR=solr.default.confdir
Review Comment:
I'm not seeing what you replaced that with.
##########
solr/core/src/java/org/apache/solr/core/NodeConfig.java:
##########
@@ -248,7 +248,7 @@ public static NodeConfig loadNodeConfig(Path solrHome,
Properties nodeProperties
// ZkFailoverTest test case...
String zkHost = nodeProperties.getProperty(SolrXmlConfig.ZK_HOST);
if (StrUtils.isNotNullOrEmpty(zkHost)) {
- int startUpZkTimeOut = 1000 * Integer.getInteger("waitForZk", 0);
+ int startUpZkTimeOut = 1000 * Integer.getInteger("solr.wait.for.zk", 0);
Review Comment:
no; it misses a category. "solr.cloud.wait.for.zk.seconds" (I'm proposing a
time unit while we're at it)
##########
dev-docs/ui/testing-and-deployment.adoc:
##########
@@ -127,8 +127,8 @@ more secure production configuration.
Like other modules, the new UI is enabled with the jetty module parameter
`--module=new-ui`.
When using the Solr CLI, users can disable the new UI with
-`SOLR_ADMIN_UI_EXPERIMENTAL_DISABLED=true` or by disabling the entire UI with
-`SOLR_ADMIN_UI_DISABLED=true`.
+`SOLR_ADMIN_UI_EXPERIMENTAL_ENABLED=false` or by disabling the entire UI with
+`SOLR_UI_ENABLED=false`.
Review Comment:
It seems wrong to *both* have "ADMIN_UI" and "UI". Pick one. Jan proposed
"UI" which I agree with.
--
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]
