Changing the subject line to reflect feature being discussed.... Chiradeep,
> -----Original Message----- > From: Chiradeep Vittal [mailto:chiradeep.vit...@citrix.com] > Sent: 15 August 2012 07:32 > To: CloudStack DeveloperList > Cc: Pranav Saxena; David Nalley; Vijay Venkatachalam; Alena > Prokharchyk; Deepak Garg > Subject: Re: where features are developed was: Review Request: Merge > Kelven's VPC code for Vmware into asf vpc branch > > I have a few comments: > 1. The table autoscale_vmprofiles has account_id as well as user_id. > Only > account_id should be sufficient? User_id is critical here. This is the handle used by the NetworkElement, NetScaler, to issue DeployVM and DestroyVM API calls to CloudStack management server. Column name in autoscale_vmprofiles db table is autoscale_user_id. > 2. The table 'counter' and 'condition' should be renamed so that it is > clear that it is autoscaling related. So also the corresponding Java > classes, APIs and interfaces The idea is to look at Counter and Condition as generic entities which can be used in various use cases and autoscale is one of such use case. Tomorrow once we have a robust messaging framework(per Murali's proposal) in CloudStack one could attach outbound actions like email send, SNMP event forward to a third-party SNMP manager say once a condition is evaluated to true . Hence we do not want to restrict these entities to AutoScale. Please let us know your thoughts. > 3. Any indexes at all on these new tables? > 4. autoscale_vmprofiles (and java class) should not have reference to > snmp. We discussed why before, and alternatives to this design on this > mailing list. > 5. A lot of the changes to existing files are whitespace changes that > are > unrelated to the logic. Please avoid this. > 6. Why is a Counter not a ControlledEntity but Condition is? > 7. Not sure why the LoadBalancer* classes have been touched. Everything > should be isolated to Autoscale? Can you explain why you had to make > API > changes to the LoadBalancingRulesManager? It is a Loadbalancer rule which is enabled/configured for AutoScale. The changes in Loadbalancer* classes are only to bind the AutoScale feature to an Loadbalancer rule. Rest all entities such as Counter, Conditions, AutoScaleProfile etc reside outside Loadbalancer* classes. > 8. The License header is not consistent (some with leading spaces, > others > without) > 9. Not all files have the Apache license. > Thanks, Ram
