----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20401/#review40489 -----------------------------------------------------------
Good job with the test coverage. Mostly minor comments. helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java <https://reviews.apache.org/r/20401/#comment73516> Use BaseDataAccessor<ZNRecord> baseAccessor = ... helix-core/src/main/java/org/apache/helix/model/IdealState.java <https://reviews.apache.org/r/20401/#comment73526> I would call this HELIX_ENABLED to stay consistent with instance enable/disable helix-core/src/main/java/org/apache/helix/model/IdealState.java <https://reviews.apache.org/r/20401/#comment73518> I would fold this into a single setEnabled(boolean) method to be symmetric with the instance enable behavior helix-core/src/main/java/org/apache/helix/participant/HelixCustomCodeRunner.java <https://reviews.apache.org/r/20401/#comment73527> Add a javadoc explaining why this method is useful. helix-core/src/test/java/org/apache/helix/integration/TestDisableCustomCodeRunner.java <https://reviews.apache.org/r/20401/#comment73529> Differentiate between init/callback and finalize helix-core/src/test/java/org/apache/helix/integration/TestDisableResource.java <https://reviews.apache.org/r/20401/#comment73532> You might want to verify by polling here since external view can change multiple times. helix-core/src/test/java/org/apache/helix/manager/zk/TestZkHelixAdmin.java <https://reviews.apache.org/r/20401/#comment73533> HelixAdmin tool = ... - Kanak Biscuitwala On April 15, 2014, 6:35 p.m., Zhen Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20401/ > ----------------------------------------------------------- > > (Updated April 15, 2014, 6:35 p.m.) > > > Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna. > > > Bugs: HELIX-94 > > > Repository: helix-git > > > Description > ------- > > [HELIX-94] Add the ability to enable and disable a resource > > > Diffs > ----- > > > helix-admin-webapp/src/main/java/org/apache/helix/webapp/resources/JsonParameters.java > c8e8fc6 > > helix-admin-webapp/src/main/java/org/apache/helix/webapp/resources/ResourceGroupResource.java > 365b212 > > helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestDisableResource.java > e69de29 > helix-core/src/main/java/org/apache/helix/HelixAdmin.java d5c62fa > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/AutoRebalancer.java > 745a9c9 > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/CustomRebalancer.java > 69037d9 > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/SemiAutoRebalancer.java > 420e7ab > > helix-core/src/main/java/org/apache/helix/controller/rebalancer/util/ConstraintBasedAssignment.java > 3fd52f4 > helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java > 169b2bb > helix-core/src/main/java/org/apache/helix/model/IdealState.java 7a4fcad > > helix-core/src/main/java/org/apache/helix/participant/HelixCustomCodeRunner.java > 6a2490a > helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java a39e571 > > helix-core/src/test/java/org/apache/helix/controller/strategy/TestAutoRebalanceStrategy.java > c93c51e > > helix-core/src/test/java/org/apache/helix/integration/TestDisableCustomCodeRunner.java > e69de29 > > helix-core/src/test/java/org/apache/helix/integration/TestDisableResource.java > e69de29 > helix-core/src/test/java/org/apache/helix/manager/zk/TestZkHelixAdmin.java > 4b3764f > helix-core/src/test/java/org/apache/helix/tools/TestClusterSetup.java > 7190968 > > Diff: https://reviews.apache.org/r/20401/diff/ > > > Testing > ------- > > > Thanks, > > Zhen Zhang > >
