Thanks for the feedback Kanak, I will look at it tomorrow as soon as I get a chance. If you want an idea on how all of this would look like take a look at the HelixClusterBuilderFactory, it might give you an idea of the direction. If not point me to an existing example and I will work towards it.
I am trying to work through the Commands now so we can start adding them under HelixAdmin, but I want to work with each command and get you guys to review it. The first cut is with HelixClusterCommand, so please do take a look at it. Thanks, Sandeep On Sun, Apr 6, 2014 at 4:29 PM, Kanak Biscuitwala <[email protected]> wrote: > > Hi Sandeep, > > I have replied inline to your commits (as best I can considering I'm not at > my desk right now :)). High-level, it looks good, but I really would like to > see a class using all this to be sure that it makes sense, even if it can't > compile yet. > > Kanak > > ---------------------------------------- >> Date: Sun, 6 Apr 2014 12:50:40 -0700 >> Subject: Re: API Rework: Update >> From: [email protected] >> To: [email protected] >> >> Hi all, >> >> I started working on the commands, starting with the cluster. Given >> that the Configuration hierarchy relies on the ZNRecord, I propose the >> following approach. >> >> We have the Admin API take in commands which then return the classes >> under api.model. What we can then do is move the configuration back >> into the core and split out the SPI parts from it. >> >> My approach earlier was to use the configurations as return values >> from the command calls but I think thats a larger refactor. >> >> Let me know what you guys think, as a sample I am working on the >> ClusterCommand and once I have it working will check it in as an >> example. >> >> Thanks, >> >> Sandeep >> >> On Sun, Apr 6, 2014 at 3:36 AM, Sandeep Nayak <[email protected]> wrote: >>> Hi guys, >>> >>> I have added all of the review feedback to the API. Here are the changes >>> >>> * Removed the rebalancer configuration into configuration. >>> * Added Provisioner and TargetProvider configurations. >>> * All constructors using ZNRecord is deprecated >>> * Added clusterid to memberid and resourceid >>> * Removed apis to create generic members from admin >>> * Changed all other non-participant APIs to be final and throws >>> UnsupportedException as discussed. >>> >>> I have been unable to move the ZNRecord and ZNRecordDelta and other >>> classes from model primarily because that means HelixConfig needs to >>> be moved which is currently the base class from most configurations. >>> Ideally we have the ZNRecord abstracted through SPI and not in API. >>> >>> As usual feedback is welcome. >>> >>> Thanks, >>> >>> Sandeep >
