> On July 10, 2014, 3:22 p.m., Raul Gutierrez Segales wrote:
> > src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java, line 139
> > <https://reviews.apache.org/r/23259/diff/2/?file=626641#file626641line139>
> >
> >     nit: maybe put the string & classes in a Map<String, Class> and then 
> > just call:
> >     
> >     ```
> >     testCommand("monitor", map.keys(), map.values())
> >     ```
> >     
> >     for readability?
> 
> Bill Havanki wrote:
>     I'll make that change, good idea.

A map ended up being tricky, since map.keySet() and map.values() are a Set and 
Collection (resp.) and I was worried about whether they would iterate in 
parallel. Instead I just defined a Field object for each key/type pairing and 
worked with arrays of them. It ended up very readable.


- Bill


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


On July 11, 2014, 11:36 a.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23259/
> -----------------------------------------------------------
> 
> (Updated July 11, 2014, 11:36 a.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Bugs: ZOOKEEPER-1346
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1346
> 
> 
> Repository: zookeeper-git
> 
> 
> Description
> -------
> 
> Addition of new AdminServer, with implementation using Jetty, for RESTful 
> handling of commands.
> 
> Work done by Skye Wanderman-Milne; this patch is a rebase on trunk since the 
> last patch was provided early in 2013.
> 
> Suggested ordering for review:
> 
> 1. ServerCnxn, ServerCnxnFactory and implementations.
> 2. DataTree, WatchManager, ZKDatabase.
> 3. ExpiryQueue, SessionTracker and Impl, LeaderSessionTracker, 
> LearnerSessionTracker.
> 4. ZooKeeperServer.
> 5. org.apache.zookeeper.server.admin package
> 6. Everything else (integration with admin server).
> 
> 
> Diffs
> -----
> 
>   ivy.xml ad14b80 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml bb16fa5 
>   src/java/main/org/apache/zookeeper/server/DataTree.java f2a4f86 
>   src/java/main/org/apache/zookeeper/server/ExpiryQueue.java 130c58e 
>   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 6e057a5 
>   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
> 6b8d2a6 
>   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 35ea301 
>   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java b0f8130 
>   src/java/main/org/apache/zookeeper/server/ServerStats.java aa0d93f 
>   src/java/main/org/apache/zookeeper/server/SessionTracker.java 4875ead 
>   src/java/main/org/apache/zookeeper/server/SessionTrackerImpl.java c971b89 
>   src/java/main/org/apache/zookeeper/server/WatchManager.java 39bf82e 
>   src/java/main/org/apache/zookeeper/server/WatchesPathReport.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/WatchesReport.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/WatchesSummary.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/ZKDatabase.java 91d6bce 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java bee35c0 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerConf.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 7bffaf0 
>   src/java/main/org/apache/zookeeper/server/ZooTrace.java 42417f5 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServer.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/AdminServerFactory.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/DummyAdminServer.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/admin/JettyAdminServer.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 3baf41a 
>   src/java/main/org/apache/zookeeper/server/quorum/LeaderSessionTracker.java 
> e746892 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java 43e1b8b 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java 
> cf0ecfb 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 4ec1f9a 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 
> b374c29 
>   
> src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java 
> 0369eb9 
>   src/java/test/org/apache/zookeeper/ZKTestCase.java 6ecfe8f 
>   src/java/test/org/apache/zookeeper/server/PrepRequestProcessorTest.java 
> b1b44fb 
>   src/java/test/org/apache/zookeeper/server/WatchesPathReportTest.java 
> PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/WatchesReportTest.java 
> PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/WatchesSummaryTest.java 
> PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/ZooKeeperServerConfTest.java 
> PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/admin/CommandsTest.java 
> PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/admin/JettyAdminServerTest.java 
> PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java 
> b04d2bd 
>   src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 
> f35e1cd 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 0ea7f19 
> 
> Diff: https://reviews.apache.org/r/23259/diff/
> 
> 
> Testing
> -------
> 
> Compiles, unit tests pass. Tested commands "by hand" on running instance.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>

Reply via email to