Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/179#discussion_r101572026
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml ---
    @@ -1155,6 +1155,30 @@ server.3=zoo3:2888:3888</programlisting>
                 </listitem>
               </varlistentry>
     
    +          <varlistentry>
    +            <term>4lw.commands.whitelist</term>
    +
    +            <listitem>
    +              <para>(Java system property: <emphasis
    --- End diff --
    
    This new configuration option is provided as *both* zoo.cfg option and 
system properties so users can encode the white list in zoo.cfg and that is the 
recommended approach as documented in the admin manual, so I don't see why 
users have to "passing via command line options".
    
    Let's see if we don't make it as a system property - then what need change:
    * Implementation is complicated. We have to add additional logic in 
QuorumPeerConfig to parse this option explicitly. Right now it naturally fall 
through the generic system property parsing code.
    * Unit tests are harder to write. We'd need a way to set these values in 
unit tests and that implies we either have to add additional setter methods, or 
have to create zoo.cfg manually which is cumbersome.
    
    Thus I tend to keep the current implementation unless there is a strong 
reason not to make this also available as a system property.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to