----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13878/ -----------------------------------------------------------
(Updated Aug. 28, 2013, 5:14 p.m.) Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna. Changes ------- add mapper classes Bugs: HELIX-109 Repository: helix-git Description ------- commit 394e06a2894f92b0a9fb2787a57d3a0ef7468825 Author: zzhang <[email protected]> Date: Tue Aug 27 18:03:33 2013 -0700 [HELIX-109] Review Helix model package, initial changes :000000 100644 0000000... f92b14f... A helix-core/src/main/java/org/apache/helix/api/Cluster.java :000000 100644 0000000... 3968376... A helix-core/src/main/java/org/apache/helix/api/ClusterId.java :000000 100644 0000000... 643e466... A helix-core/src/main/java/org/apache/helix/api/ClusterReader.java :000000 100644 0000000... bab45f8... A helix-core/src/main/java/org/apache/helix/api/ClusterSnapshot.java :000000 100644 0000000... 5266a81... A helix-core/src/main/java/org/apache/helix/api/Controller.java :000000 100644 0000000... 1ff3bd3... A helix-core/src/main/java/org/apache/helix/api/ControllerId.java :000000 100644 0000000... 479c33b... A helix-core/src/main/java/org/apache/helix/api/ControllerSnapshot.java :000000 100644 0000000... e66fb7a... A helix-core/src/main/java/org/apache/helix/api/CurState.java :000000 100644 0000000... 3a5d4ac... A helix-core/src/main/java/org/apache/helix/api/ExtView.java :000000 100644 0000000... 84d0a8f... A helix-core/src/main/java/org/apache/helix/api/HelixVersion.java :000000 100644 0000000... d65c3d7... A helix-core/src/main/java/org/apache/helix/api/Id.java :000000 100644 0000000... ac7638d... A helix-core/src/main/java/org/apache/helix/api/Msg.java :000000 100644 0000000... c61a5bf... A helix-core/src/main/java/org/apache/helix/api/MsgId.java :000000 100644 0000000... 16ed316... A helix-core/src/main/java/org/apache/helix/api/Participant.java :000000 100644 0000000... eadc615... A helix-core/src/main/java/org/apache/helix/api/ParticipantId.java :000000 100644 0000000... 48bdfff... A helix-core/src/main/java/org/apache/helix/api/ParticipantSnapshot.java :000000 100644 0000000... c903493... A helix-core/src/main/java/org/apache/helix/api/Partition.java :000000 100644 0000000... ace767d... A helix-core/src/main/java/org/apache/helix/api/PartitionId.java :000000 100644 0000000... e244032... A helix-core/src/main/java/org/apache/helix/api/ProcId.java :000000 100644 0000000... 3d1c69f... A helix-core/src/main/java/org/apache/helix/api/RebalancerRef.java :000000 100644 0000000... dc615de... A helix-core/src/main/java/org/apache/helix/api/Resource.java :000000 100644 0000000... 32cdf4f... A helix-core/src/main/java/org/apache/helix/api/ResourceId.java :000000 100644 0000000... a5e04ff... A helix-core/src/main/java/org/apache/helix/api/ResourceSnapshot.java :000000 100644 0000000... a7b6316... A helix-core/src/main/java/org/apache/helix/api/RscAssignment.java :000000 100644 0000000... 49c5ccf... A helix-core/src/main/java/org/apache/helix/api/RunningInstance.java :000000 100644 0000000... 9a070c0... A helix-core/src/main/java/org/apache/helix/api/SessionId.java :000000 100644 0000000... a4ab2c5... A helix-core/src/main/java/org/apache/helix/api/Spectator.java :000000 100644 0000000... bd0150c... A helix-core/src/main/java/org/apache/helix/api/SpectatorId.java :000000 100644 0000000... 947c767... A helix-core/src/main/java/org/apache/helix/api/SpectatorSnapshot.java :000000 100644 0000000... ebd31cc... A helix-core/src/main/java/org/apache/helix/api/State.java :000000 100644 0000000... fe2b3e0... A helix-core/src/main/java/org/apache/helix/api/StateModelDefId.java close the other jira. paste the original comments here: can we have separate packages for model, role, listeners ? Lets avoid having strings, even things like cluster name, resource name, partition name should have concrete classes. Also having builder for every object is helpful. what do you think Kanak Biscuitwala 3 days, 19 hours ago (Aug. 24, 2013, 5:50 a.m.) - Package separation makes sense. Currently, there's only model and role classes. I think a new discussion is necessary to decide how to do listeners right (and more broadly, concrete use cases for this new API). - I agree with the string comment. Many (if not all) of the Map<String, Obj> could be List/Set<Obj> instead and others can be Map<Obj1, Obj2>. - I think the following can be candidates for builders: Cluster, Resource, Participant, and maybe CurState and ExtView. I can create a separate Jira and work on those. I'm sure Jason has additional thoughts. Zhen Zhang 2 days, 20 hours ago (Aug. 25, 2013, 4:56 a.m.) Some comments about the 2nd point: using Map instead of List/Set might be convenient since in many cases, we need to index ideal-state/current-state/message by id's. Agree to use Id instead of String, but it will be just a wrapper around string and we need to override equal() and hash() methods. To avoid code duplication, we may have an Id base class and ResourceId, ClusterId, ParticipantId, PartitionId, etc will be empty derived classes from Id class? Diffs (updated) ----- helix-core/src/main/java/org/apache/helix/api/ClusterMapper.java e69de29 helix-core/src/main/java/org/apache/helix/api/CurStateMapper.java e69de29 helix-core/src/main/java/org/apache/helix/api/ExtViewMapper.java e69de29 helix-core/src/main/java/org/apache/helix/api/ParticipantMapper.java e69de29 helix-core/src/main/java/org/apache/helix/api/ResourceMapper.java e69de29 Diff: https://reviews.apache.org/r/13878/diff/ Testing ------- Thanks, Zhen Zhang
