Awesome work, I am review the api package. Can you provide some additional info on commons-discovery ?. Is this short term or long term, whats the value add.
Initial code structure looks good package looks good. Agree with I* classes. We should bring all model classes here. I did see some formatting issues. I think Kanak has a eclipse style xml that fixes the formatting issues. Not a big issue. To setup eclipse, we need to first run mvn install -DskipTests. What are the next steps. If I understand correctly we will move model and listener api's. I think we should change the listener interfaces and create event classes. Also we have overloaded INIT and FINALIZE callback that has caused some issues. Do we have a JIRA, we can add things we want to fix as subTasks and we revisit later. thanks, Kishore G On Sat, Mar 1, 2014 at 4:53 PM, Sandeep Nayak <[email protected]> wrote: > Hey guys, > > I checked in some work into the branch > https://github.com/kanakb/helix. Here is a summary > > (1) I have ZNRecord and ZNRecordDelta are kept as is in api. I think > we need to revisit them and rework them > > (2) I have all ids retained with the json deserialization info. > Removing that would have been a lot of rework and I was focussed on > moving files into API this first go around. We need to clean up the > @Json annotations > > (3) I have a model package but I decided to hold off on moving all > models into that package because there were some internal > dependencies, so I opted to use interfaces for the time being until we > untangle some of the dependencies of model on core at which point we > can drop the interfaces and move the model into api cleanly. So bear > with those IInterface naming conventions, its an eyesore for a reason. > > (4) Please note the builder pattern I introduced, there are abstract > builders in API and the implementations are in core. I decided to > avoid writing plumbing and get something quick going so leveraged > apache commons discovery. You can see the pattern in the builder > packages in api. > > (5) API has third party dependencies on google-guava, > apache-commons-discovery and jackson. We need to bring this down to a > very minimum as we work through the API cleanup > > (6) I added a SerializableZNRecord in org.apache.helix in core because > I did not want serializer references in the api package. It felt like > an implementation detail > > (7) I added a AbstractRebalancerConfig in > org.apache.helix.controller.rebalancer.config package in core. Reason > was the serializer references again. > > (8) Most apis use <T extends interface> pattern so that we dont have > to cast objects in the core project. > > (9) Note the ids classes are final deliberately. I would like to keep > the constructors private but they are accessed in some places so had > to keep them open. > > (10) I moved most of the enums into api. > > (11) StateModelDefId was referencing an internal class for a constant > "SchedulerTaskQueue", I moved that into api and now everywhere the > core class was referenced I reference the constant id value. > > I think I got everything, there is still a lot to be done but I did > not want to keep going off the deep end and wanted to give everyone a > chance to review, so we can rework as necessary. > > Apologies if I have missed out on formatting in some places, I tried > to do a thorough job but I am sure I have fallen short in places. It > has been quite some time since I worked on maven builds so let me know > if I missed anything in the maven builds. > > Thanks, > > Sandeep >
