You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "epugh (via GitHub)" <gi...@apache.org> on 2025/09/24 22:04:21 UTC

[PR] SOLR-17864: Migrate our variable to less problematic language [solr]

epugh opened a new pull request, #3692:
URL: https://github.com/apache/solr/pull/3692

   https://issues.apache.org/jira/browse/SOLR-17864
   
   Unfortunantly the underlying system property is defined by the Zookeeper project, so we can't change that.
   
   I thought originally I could just use our EnvUtils and map the old name to the new name, but I don't think that will work.  So I just renamed the variable in Solr land..
   
   Question: Does this indirection make the code harder to read and maybe not worth it?
   


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-17864: Migrate our variable to less problematic language [solr]

Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #3692:
URL: https://github.com/apache/solr/pull/3692#discussion_r2440040717


##########
solr/core/src/java/org/apache/solr/cloud/SolrZkServer.java:
##########
@@ -40,7 +40,8 @@
 public class SolrZkServer {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-  public static final String ZK_WHITELIST_PROPERTY = "zookeeper.4lw.commands.whitelist";
+  // We do not have control over how Zookeeper names it's properties.

Review Comment:
   Wondering if it might be change-able over time ... https://github.com/apache/zookeeper/blob/release-3.9.4/zookeeper-server/src/main/java/org/apache/zookeeper/server/command/FourLetterCommands.java#L150



-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-17864: Migrate our variable to less problematic language [solr]

Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #3692:
URL: https://github.com/apache/solr/pull/3692#discussion_r2440054595


##########
solr/core/src/java/org/apache/solr/cloud/SolrZkServer.java:
##########
@@ -138,8 +139,8 @@ public void start() {
       return;
     }
 
-    if (System.getProperty(ZK_WHITELIST_PROPERTY) == null) {
-      System.setProperty(ZK_WHITELIST_PROPERTY, "ruok, mntr, conf");
+    if (EnvUtils.getProperty(ZK_ALLOWLIST_PROPERTY) == null) {

Review Comment:
   From the PR description it sounds that perhaps you meant to change this back?
   ```suggestion
       if (System.getProperty(ZK_ALLOWLIST_PROPERTY) == null) {
   ```



-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-17864: Migrate our variable to less problematic language [solr]

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh closed pull request #3692: SOLR-17864: Migrate our variable to less problematic language
URL: https://github.com/apache/solr/pull/3692


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-17864: Migrate our variable to less problematic language [solr]

Posted by "psalagnac (via GitHub)" <gi...@apache.org>.
psalagnac commented on code in PR #3692:
URL: https://github.com/apache/solr/pull/3692#discussion_r2379584544


##########
solr/core/src/java/org/apache/solr/cloud/SolrZkServer.java:
##########
@@ -138,8 +139,8 @@ public void start() {
       return;
     }
 
-    if (System.getProperty(ZK_WHITELIST_PROPERTY) == null) {
-      System.setProperty(ZK_WHITELIST_PROPERTY, "ruok, mntr, conf");
+    if (EnvUtils.getProperty(ZK_ALLOWLIST_PROPERTY) == null) {

Review Comment:
   Not sure we want to use `EnvUtil` there. It checks environment variables, but I think Zookeeper code will work only with a system property.
   
   Probably a case will never happen anyway...



-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-17864: Migrate our variable to less problematic language [solr]

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3692:
URL: https://github.com/apache/solr/pull/3692#issuecomment-3481611061

   Closing this, as fixing it in Solr but not in ZK seems like it's confusing and doesn't accomplish our goal.


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-17864: Migrate our variable to less problematic language [solr]

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #3692:
URL: https://github.com/apache/solr/pull/3692#issuecomment-3415772785

   I think this PR is probably going to just time out...   As in eventually solrbot will mark it stale and then close it, because the effort of fixing the language in the ZK project is probably too high for me ;-(.


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


Re: [PR] SOLR-17864: Migrate our variable to less problematic language [solr]

Posted by "chatman (via GitHub)" <gi...@apache.org>.
chatman commented on PR #3692:
URL: https://github.com/apache/solr/pull/3692#issuecomment-3330868435

   I think this is not worth the confusion. I don't think it is "problematic language", since we're not in a speech mode, but merely trying to adhere to using an external dependency the way it is supposed to be used. 


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org