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 >
