zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stevo Slavic (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (ZOOKEEPER-2272) Code clean up in ZooKeeperServer and KerberosName
Date Wed, 22 Feb 2017 09:45:45 GMT

    [ https://issues.apache.org/jira/browse/ZOOKEEPER-2272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15877868#comment-15877868

Stevo Slavic commented on ZOOKEEPER-2272:

There should be more thorough cleanup of configuration management code and documentation.
Seems every class implements it's own way of obtaining configuration and as consequence there's
lots of inconsistency. E.g. in {{ZooKeeperServer}} __allowSaslFailedClients__ is __true__/__false__,
while in {{PrepRequestProcessor}} __skipACL__ is __yes__/__no__
Some properties are defined via config file, some via system properties but not both. Because
of lack of consistency or even proper configuration design, that inconsistency is reflected
in poor documentation as well.

For good configuration design, implementation and documentation ideas please have a look at
Apache Kafka (especially bit where they generate configuration documentation from source),
Apache Commons Configuration ({{AbstractConfiguration}} abstraction, extended in Netflix Archaius
Core with {{DynamicPropertyFactory}}), and Spring framework {{PropertySource}} abstraction.

> Code clean up in ZooKeeperServer and KerberosName
> -------------------------------------------------
>                 Key: ZOOKEEPER-2272
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2272
>             Project: ZooKeeper
>          Issue Type: Improvement
>          Components: server
>    Affects Versions: 3.5.0
>            Reporter: Mohammad Arshad
>            Assignee: Mohammad Arshad
>            Priority: Trivial
>             Fix For: 3.5.3
>         Attachments: ZOOKEEPER-2272-01.patch
> # Following code in {{org.apache.zookeeper.server.ZooKeeperServer}} should be cleaned
up. Some how it got missed in code review
> {code}
> if ((System.getProperty("zookeeper.allowSaslFailedClients") != null)
>   &&  (System.getProperty("zookeeper.allowSaslFailedClients").equals("true")))
> {code}
> should be replaced with 
> {code}
> if(Boolean.getBoolean("zookeeper.allowSaslFailedClients"))
> {code}
> # Similar code clean up can be done in {{org.apache.zookeeper.server.auth.KerberosName}}

This message was sent by Atlassian JIRA

View raw message