[jira] [Commented] (ZOOKEEPER-2272) Code clean up in ZooKeeperServer and KerberosName
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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
(v6.3.15#6346)
[jira] [Commented] (ZOOKEEPER-2272) Code clean up in ZooKeeperServer and KerberosName
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15342916#comment-15342916
]
Hadoop QA commented on ZOOKEEPER-2272:
--
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12755079/ZOOKEEPER-2272-01.patch
against trunk revision 1748630.
+1 @author. The patch does not contain any @author tags.
-1 tests included. The patch doesn't appear to include any new or modified
tests.
Please justify why no new tests are needed for this
patch.
Also please list what manual steps were performed to
verify this patch.
-1 patch. The patch command could not apply the patch.
Console output:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3209//console
This message is automatically generated.
> 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: Arshad Mohammad
>Assignee: Arshad Mohammad
>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.4#6332)
[jira] [Commented] (ZOOKEEPER-2272) Code clean up in ZooKeeperServer and KerberosName
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14738331#comment-14738331
]
Hadoop QA commented on ZOOKEEPER-2272:
--
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12755079/ZOOKEEPER-2272-01.patch
against trunk revision 1702163.
+1 @author. The patch does not contain any @author tags.
-1 tests included. The patch doesn't appear to include any new or modified
tests.
Please justify why no new tests are needed for this
patch.
Also please list what manual steps were performed to
verify this patch.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac
compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3)
warnings.
+1 release audit. The applied patch does not increase the total number of
release audit warnings.
-1 core tests. The patch failed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2869//testReport/
Findbugs warnings:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2869//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2869//console
This message is automatically generated.
> 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: Arshad Mohammad
>Assignee: Arshad Mohammad
>Priority: Trivial
> Fix For: 3.5.2
>
> 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.4#6332)
[jira] [Commented] (ZOOKEEPER-2272) Code clean up in ZooKeeperServer and KerberosName
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14739209#comment-14739209
]
Arshad Mohammad commented on ZOOKEEPER-2272:
{quote}
-1 core tests. The patch failed core unit tests.
{quote}
One test case is skipped but that is not related to this patch
> 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: Arshad Mohammad
>Assignee: Arshad Mohammad
>Priority: Trivial
> Fix For: 3.5.2
>
> 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.4#6332)
