Sounds reasonable. I replied to a couple of your responses on the commit page, but looks fine overall.
Kanak ---------------------------------------- > Date: Mon, 31 Mar 2014 08:32:58 -0700 > Subject: Re: Helix Administrator Client and Helix Client strawman checked in > From: [email protected] > To: [email protected] > > I responded to some comments and noted some changes to make. Please > see responses inline for questions asked in mail. > > Keep them coming, this is a good review. > > Thanks Kanak, > > Sandeep > > On Mon, Mar 31, 2014 at 12:01 AM, Kanak Biscuitwala <[email protected]> > wrote: >> >> I added comments directly to >> https://github.com/kanakb/helix/commit/b88217a811f3d175fdc8bd0f0c58467d13de62a3 >> >> General comments: >> - What other subclasses of HelixClient are you planning? > > [Sandeep] None I am only hoping one subclass of administrator at this > point. The open question on that one is should we allow users to write > their own client extensions? > >> - It's good to plan out some of these methods, like the CRUD operations on >> spectator, controller, etc, but we shouldn't include these methods in the >> release since we don't plan to implement them right away. >> - There are a lot of classes. Do we really need all of them? > > [Sandeep] Lets get into specific classes and if we dont need them we > can discard them, this is an iterative process and not a final cut. > >> - I don't understand the relationship between HelixConnection and >> HelixClient/HelixAdministratorClient. How will they be related to each other? > [Sandeep] I had eluded to this in my mail, see 2nd * item. But to > elaborate the connection classes will move to SPI and will be held > internal to the HelixClient concrete implementations. > >> >> Kanak >> >> ---------------------------------------- >>> Date: Sun, 30 Mar 2014 21:33:56 -0700 >>> Subject: Helix Administrator Client and Helix Client strawman checked in >>> From: [email protected] >>> To: [email protected] >>> >>> Hey guys, >>> >>> I checked in a strawman for the helix administrator and helix client >>> api classes. There are command classes, id classes and model classes >>> which tie up the two client apis. The idea is mutation is only allowed >>> by Administrator API, the client API is only used for discovery. >>> >>> Please take a look and let me know of the structure and composition. I >>> decided to not go with the composition model as we had discussed i.e. >>> HelixCluster having APIs for the creation of entities under it as >>> separation between administrator client and regular clients was >>> getting clumsy to achieve. So, I decided to switch back to a >>> HelixAdministrator and HelixClient based approach which worked better. >>> >>> Here are the highlights >>> >>> * The api.role package is no longer needed and all its constituents >>> are represented in the model package. >>> >>> * The existing contents of the api.client package now seem to be >>> candidates for spi project (something I plan to start on in parallel >>> soon). In other words HelixConnection, HelixConnectionBuilder, >>> HelixConnectionFactory, HelixConnectionFactoryListener seem to be >>> internal concerns to the implementations of the API. >>> >>> * If the structure looks good, I will next start on fleshing out the >>> contents of each of the command classes. I presume there should be >>> existing code I can get information from so I will start there when I >>> get a chance. I will need help on the commands, so will ping at the >>> appropriate time. >>> >>> * The contents of api.model package like ZNRecord, ZNRecordDelta etc >>> will move under the implementation in core. I will keep >>> HelixConfiguration and have configurations derive from it and these >>> will eventually get translated before storing into the specific >>> implementation. >>> >>> As usual this is work in progress but wanted to give early visibility >>> so feedback, comments are welcome. >>> >>> Thanks, >>> >>> Sandeep >>
