Re: Review Request 24332: [HELIX-484] Remove CallbackHandler/ZkCallbackHandler code duplication, [HELIX-486] Remove StateModelFactory/HelixStateModelFactory code duplication

2014-08-07 Thread Kanak Biscuitwala

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

Ship it!


Ship It!

- Kanak Biscuitwala


On Aug. 7, 2014, 10:50 a.m., Zhen Zhang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24332/
 ---
 
 (Updated Aug. 7, 2014, 10:50 a.m.)
 
 
 Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna.
 
 
 Bugs: HELIX-484 and HELIX-486
 
 
 Repository: helix-git
 
 
 Description
 ---
 
 Remove CallbackHandler/ZkCallbackHandler code duplication
 Remove StateModelFactory/HelixStateModelFactory code duplication
 
 
 Diffs
 -
 
   
 helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetInstance.java
  a9ecaa0 
   
 helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetResource.java
  a54b0a3 
   helix-agent/src/main/java/org/apache/helix/agent/AgentStateModel.java 
 d227ac3 
   
 helix-agent/src/main/java/org/apache/helix/agent/AgentStateModelFactory.java 
 69d45ae 
   
 helix-core/src/main/java/org/apache/helix/api/StateTransitionHandlerFactory.java
  45f56e5 
   helix-core/src/main/java/org/apache/helix/api/TransitionHandler.java 
 9717340 
   helix-core/src/main/java/org/apache/helix/api/id/StateModelDefId.java 
 7c84f0f 
   
 helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java 
 6aa3ab9 
   
 helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixParticipant.java 
 af50eb7 
   
 helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java
  1bb6506 
   
 helix-core/src/main/java/org/apache/helix/participant/CustomCodeInvoker.java 
 6c96629 
   
 helix-core/src/main/java/org/apache/helix/participant/DistClusterControllerStateModel.java
  0c2eb7c 
   
 helix-core/src/main/java/org/apache/helix/participant/DistClusterControllerStateModelFactory.java
  a367c81 
   
 helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyModel.java
  3866cf5 
   
 helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyStateModelFactory.java
  51c91cc 
   
 helix-core/src/main/java/org/apache/helix/participant/HelixCustomCodeRunner.java
  2f169cc 
   
 helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java
  95afb70 
   
 helix-core/src/main/java/org/apache/helix/participant/StateMachineEngine.java 
 abb7d81 
   
 helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModel.java
  ca67d42 
   
 helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModelFactory.java
  a205910 
   
 helix-core/src/main/java/org/apache/helix/participant/statemachine/StateModelParser.java
  eddeaa5 
   helix-core/src/main/java/org/apache/helix/task/TaskRunner.java 66abba6 
   helix-core/src/main/java/org/apache/helix/task/TaskStateModel.java a44a8cb 
   helix-core/src/main/java/org/apache/helix/task/TaskStateModelFactory.java 
 2537747 
   helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 6599b33 
   helix-core/src/test/java/org/apache/helix/DummyProcessThread.java f51aa1d 
   helix-core/src/test/java/org/apache/helix/Mocks.java 0303f12 
   helix-core/src/test/java/org/apache/helix/TestHelixTaskExecutor.java 
 a3b16e5 
   helix-core/src/test/java/org/apache/helix/TestHelixTaskHandler.java 43b4407 
   helix-core/src/test/java/org/apache/helix/TestHelper.java 879e727 
   
 helix-core/src/test/java/org/apache/helix/integration/TestAddStateModelFactoryAfterConnect.java
  5f37845 
   
 helix-core/src/test/java/org/apache/helix/integration/TestBatchMessageWrapper.java
  6a6837a 
   
 helix-core/src/test/java/org/apache/helix/integration/TestCorrectnessOnConnectivityLoss.java
  abb2a7b 
   
 helix-core/src/test/java/org/apache/helix/integration/TestErrorPartition.java 
 19af9a7 
   
 helix-core/src/test/java/org/apache/helix/integration/TestHelixConnection.java
  3d02ae8 
   
 helix-core/src/test/java/org/apache/helix/integration/TestMessageThrottle2.java
  496a16f 
   
 helix-core/src/test/java/org/apache/helix/integration/TestMessagingService.java
  08954e5 
   
 helix-core/src/test/java/org/apache/helix/integration/TestMultiClusterController.java
  c2f9a5c 
   
 helix-core/src/test/java/org/apache/helix/integration/TestNonOfflineInitState.java
  105633a 
   
 helix-core/src/test/java/org/apache/helix/integration/TestPartitionLevelTransitionConstraint.java
  823a9ce 
   
 helix-core/src/test/java/org/apache/helix/integration/TestPreferenceListAsQueue.java
  06a2b56 
   
 helix-core/src/test/java/org/apache/helix/integration/TestResetInstance.java 
 5804744 
   
 helix-core/src/test/java/org/apache/helix/integration/TestResetPartitionState.java
  4855b3d 
   
 

Review Request 24332: [HELIX-484] Remove CallbackHandler/ZkCallbackHandler code duplication, [HELIX-486] Remove StateModelFactory/HelixStateModelFactory code duplication

2014-08-05 Thread Zhen Zhang

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

Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna.


Bugs: HELIX-484 and HELIX-486


Repository: helix-git


Description
---

Remove CallbackHandler/ZkCallbackHandler code duplication
Remove StateModelFactory/HelixStateModelFactory code duplication


Diffs
-

  
helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetInstance.java 
a9ecaa0 
  
helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetResource.java 
a54b0a3 
  helix-core/src/main/java/org/apache/helix/api/id/StateModelDefId.java 7c84f0f 
  helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java 
6aa3ab9 
  helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixParticipant.java 
af50eb7 
  
helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java
 1bb6506 
  helix-core/src/main/java/org/apache/helix/participant/CustomCodeInvoker.java 
6c96629 
  
helix-core/src/main/java/org/apache/helix/participant/DistClusterControllerStateModelFactory.java
 a367c81 
  
helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyModel.java
 3866cf5 
  
helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyStateModelFactory.java
 51c91cc 
  
helix-core/src/main/java/org/apache/helix/participant/HelixCustomCodeRunner.java
 2f169cc 
  
helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java
 95afb70 
  helix-core/src/main/java/org/apache/helix/participant/StateMachineEngine.java 
abb7d81 
  
helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModel.java
 ca67d42 
  
helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModelFactory.java
 a205910 
  helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 6599b33 
  helix-core/src/test/java/org/apache/helix/DummyProcessThread.java f51aa1d 
  helix-core/src/test/java/org/apache/helix/TestHelixTaskExecutor.java a3b16e5 
  helix-core/src/test/java/org/apache/helix/TestHelixTaskHandler.java 43b4407 
  helix-core/src/test/java/org/apache/helix/TestHelper.java 879e727 
  
helix-core/src/test/java/org/apache/helix/integration/TestAddStateModelFactoryAfterConnect.java
 5f37845 
  
helix-core/src/test/java/org/apache/helix/integration/TestBatchMessageWrapper.java
 6a6837a 
  helix-core/src/test/java/org/apache/helix/integration/TestErrorPartition.java 
19af9a7 
  
helix-core/src/test/java/org/apache/helix/integration/TestMessageThrottle2.java 
496a16f 
  
helix-core/src/test/java/org/apache/helix/integration/TestMessagingService.java 
08954e5 
  
helix-core/src/test/java/org/apache/helix/integration/TestMultiClusterController.java
 c2f9a5c 
  
helix-core/src/test/java/org/apache/helix/integration/TestNonOfflineInitState.java
 105633a 
  
helix-core/src/test/java/org/apache/helix/integration/TestPartitionLevelTransitionConstraint.java
 823a9ce 
  
helix-core/src/test/java/org/apache/helix/integration/TestPreferenceListAsQueue.java
 06a2b56 
  helix-core/src/test/java/org/apache/helix/integration/TestResetInstance.java 
5804744 
  
helix-core/src/test/java/org/apache/helix/integration/TestResetPartitionState.java
 4855b3d 
  helix-core/src/test/java/org/apache/helix/integration/TestResetResource.java 
7d28931 
  
helix-core/src/test/java/org/apache/helix/integration/TestSchedulerMessage.java 
89af602 
  
helix-core/src/test/java/org/apache/helix/integration/TestStateTransitionTimeout.java
 c4304b0 
  helix-core/src/test/java/org/apache/helix/integration/TestZkReconnect.java 
18f6fd7 
  
helix-core/src/test/java/org/apache/helix/integration/manager/TestConsecutiveZkSessionExpiry.java
 99986ef 
  
helix-core/src/test/java/org/apache/helix/integration/manager/TestHelixMultiClusterController.java
 18234b5 
  
helix-core/src/test/java/org/apache/helix/integration/manager/TestParticipantManager.java
 309ab18 
  
helix-core/src/test/java/org/apache/helix/integration/manager/TestStateModelLeak.java
 d6d7bab 
  
helix-core/src/test/java/org/apache/helix/manager/zk/MockMultiClusterController.java
 7f8b1a3 
  helix-core/src/test/java/org/apache/helix/manager/zk/MockParticipant.java 
f107d3d 
  
helix-core/src/test/java/org/apache/helix/manager/zk/TestDefaultControllerMsgHandlerFactory.java
 8b5b30c 
  helix-core/src/test/java/org/apache/helix/messaging/TestAsyncCallbackSvc.java 
da686fe 
  helix-core/src/test/java/org/apache/helix/mock/participant/DummyProcess.java 
9880605 
  
helix-core/src/test/java/org/apache/helix/mock/participant/MockBootstrapModelFactory.java
 177e7c4 
  
helix-core/src/test/java/org/apache/helix/mock/participant/MockMSModelFactory.java
 9325934 
  
helix-core/src/test/java/org/apache/helix/mock/participant/MockSchemataModelFactory.java
 525e764 
  

Re: Review Request 24332: [HELIX-484] Remove CallbackHandler/ZkCallbackHandler code duplication, [HELIX-486] Remove StateModelFactory/HelixStateModelFactory code duplication

2014-08-05 Thread Kishore Gopalakrishna

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


Instead of adding HelixStateModelFactory can we rename this class to 
StateTransitionHandlerFactory and StateModel to TransitionHandler and move it 
to api? Adding Helix prefix will create more confusion.


helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java
https://reviews.apache.org/r/24332/#comment86879

what is the equivalent of removing statemodel



helix-core/src/main/java/org/apache/helix/participant/StateMachineEngine.java
https://reviews.apache.org/r/24332/#comment86880

so we are breaking backwards compatibility?


- Kishore Gopalakrishna


On Aug. 5, 2014, 6:52 p.m., Zhen Zhang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/24332/
 ---
 
 (Updated Aug. 5, 2014, 6:52 p.m.)
 
 
 Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna.
 
 
 Bugs: HELIX-484 and HELIX-486
 
 
 Repository: helix-git
 
 
 Description
 ---
 
 Remove CallbackHandler/ZkCallbackHandler code duplication
 Remove StateModelFactory/HelixStateModelFactory code duplication
 
 
 Diffs
 -
 
   
 helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetInstance.java
  a9ecaa0 
   
 helix-admin-webapp/src/test/java/org/apache/helix/webapp/TestResetResource.java
  a54b0a3 
   helix-core/src/main/java/org/apache/helix/api/id/StateModelDefId.java 
 7c84f0f 
   
 helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java 
 6aa3ab9 
   
 helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixParticipant.java 
 af50eb7 
   
 helix-core/src/main/java/org/apache/helix/messaging/handling/HelixStateTransitionHandler.java
  1bb6506 
   
 helix-core/src/main/java/org/apache/helix/participant/CustomCodeInvoker.java 
 6c96629 
   
 helix-core/src/main/java/org/apache/helix/participant/DistClusterControllerStateModelFactory.java
  a367c81 
   
 helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyModel.java
  3866cf5 
   
 helix-core/src/main/java/org/apache/helix/participant/GenericLeaderStandbyStateModelFactory.java
  51c91cc 
   
 helix-core/src/main/java/org/apache/helix/participant/HelixCustomCodeRunner.java
  2f169cc 
   
 helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java
  95afb70 
   
 helix-core/src/main/java/org/apache/helix/participant/StateMachineEngine.java 
 abb7d81 
   
 helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModel.java
  ca67d42 
   
 helix-core/src/main/java/org/apache/helix/participant/statemachine/ScheduledTaskStateModelFactory.java
  a205910 
   helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 6599b33 
   helix-core/src/test/java/org/apache/helix/DummyProcessThread.java f51aa1d 
   helix-core/src/test/java/org/apache/helix/TestHelixTaskExecutor.java 
 a3b16e5 
   helix-core/src/test/java/org/apache/helix/TestHelixTaskHandler.java 43b4407 
   helix-core/src/test/java/org/apache/helix/TestHelper.java 879e727 
   
 helix-core/src/test/java/org/apache/helix/integration/TestAddStateModelFactoryAfterConnect.java
  5f37845 
   
 helix-core/src/test/java/org/apache/helix/integration/TestBatchMessageWrapper.java
  6a6837a 
   
 helix-core/src/test/java/org/apache/helix/integration/TestErrorPartition.java 
 19af9a7 
   
 helix-core/src/test/java/org/apache/helix/integration/TestMessageThrottle2.java
  496a16f 
   
 helix-core/src/test/java/org/apache/helix/integration/TestMessagingService.java
  08954e5 
   
 helix-core/src/test/java/org/apache/helix/integration/TestMultiClusterController.java
  c2f9a5c 
   
 helix-core/src/test/java/org/apache/helix/integration/TestNonOfflineInitState.java
  105633a 
   
 helix-core/src/test/java/org/apache/helix/integration/TestPartitionLevelTransitionConstraint.java
  823a9ce 
   
 helix-core/src/test/java/org/apache/helix/integration/TestPreferenceListAsQueue.java
  06a2b56 
   
 helix-core/src/test/java/org/apache/helix/integration/TestResetInstance.java 
 5804744 
   
 helix-core/src/test/java/org/apache/helix/integration/TestResetPartitionState.java
  4855b3d 
   
 helix-core/src/test/java/org/apache/helix/integration/TestResetResource.java 
 7d28931 
   
 helix-core/src/test/java/org/apache/helix/integration/TestSchedulerMessage.java
  89af602 
   
 helix-core/src/test/java/org/apache/helix/integration/TestStateTransitionTimeout.java
  c4304b0 
   helix-core/src/test/java/org/apache/helix/integration/TestZkReconnect.java 
 18f6fd7 
   
 helix-core/src/test/java/org/apache/helix/integration/manager/TestConsecutiveZkSessionExpiry.java
  99986ef