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] [Comment Edited] (ZOOKEEPER-2272) Code clean up in ZooKeeperServer and KerberosName
Date Wed, 22 Feb 2017 09:47:44 GMT

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

Stevo Slavic edited comment on ZOOKEEPER-2272 at 2/22/17 9:47 AM:
------------------------------------------------------------------

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.


was (Author: sslavic):
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
(v6.3.15#6346)

Mime
View raw message