-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8094/#review14507
-----------------------------------------------------------


Noticed some issues with configuration and documentation. The system properties 
should be moved into the config file. Also we need to document this feature and 
the configuration.

At the very least basic unit tests need to be added.

It would be good to attach the latest version of the patch to ZOOKEEPER-1346 
and "submit" the jira so that qabot can take a pass on it.



src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java
<https://reviews.apache.org/r/8094/#comment30896>

    whether or not the server is started should be made configurable. On by 
default is fine.



src/java/main/org/apache/zookeeper/server/admin/AdminServer.java
<https://reviews.apache.org/r/8094/#comment30897>

    Configurable properties such as this need to be in the config file, not a 
system property.


- Patrick Hunt


On Nov. 30, 2012, 9:01 p.m., Skye Wanderman-Milne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8094/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2012, 9:01 p.m.)
> 
> 
> Review request for zookeeper, Patrick Hunt, Camille Fournier, and Henry 
> Robinson.
> 
> 
> Description
> -------
> 
> See my comment in ZOOKEEPER-1346.
> 
> 
> This addresses bug ZOOKEEPER-1346.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Diffs
> -----
> 
>   ivy.xml fadf4f4 
>   src/java/main/org/apache/zookeeper/server/DataTree.java 0bb2317 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 8b4c46b 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> eec2f2a 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6dd509b 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java e5c6565 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 3535e1b 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java 31f2785 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 0e7c815 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java d6c0c05 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java 7bb7b2f 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 369e621 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java ac14fe2 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Command.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandBase.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/CommandOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/Commands.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JsonOutputter.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java 4d09b43 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java e8d548b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 
> 3182419 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 4e3a87d 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 
> deae926 
>   
> src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 
> d3f1492 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 
> 8665bac 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 42207e1 
> 
> Diff: https://reviews.apache.org/r/8094/diff/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> Ran in standalone mode (only option right now) and manually tried out all the 
> commands/links
> 
> 
> Thanks,
> 
> Skye Wanderman-Milne
> 
>

Reply via email to