Hi > Found that some of the AutoscalerImpl operations are invoked by directly > instantiating the AutoscalerImpl. I think this is not good. > AutoscalerImpl meant to be a web service, which needs to be accessed only > via service stubs, referring to the WSDL.
+1. This is completely against the web services model. Just noticed that org/apache/stratos/autoscaler/client/cloud/controller/CloudControllerClient.java class is creating an instance of AutoscalerImpl. Will have to fix this. Nice finding Sajith! > Another small thing to be highlight, regarding the interface naming. > Currently Autoscaler service's interface and CC 's interface naming are not > consistent. > IMO the part 'interface' should not be go in the name of the interface, its > no hard and fast rule, but I think its better to maintain the consistency, > So I would like to suggest to rename AutoScalerServiceInterface as > AutoScalerService. > > thoughts? +1 I''ll create JIRAs to track these tasks. I too think that it is important to clean up this code in a proper way. >> Also, how about in "updateX" operations if the updated object is returned >> instead of the boolean? That will make the programming in client side easy, >> otherwise it will need to do another "getX" call to retrieve the updated >> object ? +1 Yes, that would be make things easy in client side. >>> One KubernetesHost is associated with only one KubernetesGroup. Hence >>> ideally KubernetesHost class should have a reference to KubernetesGroup. I >>> mean KubernetesHost should have a KubernetesGroupId filed. >>> >>> There are some advantages of doing this. >>> >>> We need to pass only KubernetesHost object when calling >>> addKubernetesHost() method. Currently we are passing both KubernetesGroupId >>> and KubernetesHost object. >>> removeKubernetesHost() method will become more efficient. I could see >>> that you are looping through all Kubernetes Groups in-order to identify >>> which Group has the given host. Then you are removing the host from that >>> group. If you store the KubernetesGroupId in the KubernetesHost object, >>> then no need to loop through all the KubernetesGroups. Of course we need to >>> pass the KubernetesHost object to the removeKubernetesHost() method. >>> updateKubernetesHost() would work faster because of the same reason as >>> above. >>> >>> It will improve the performance. IMO, doing that will only add more complexity to the code. I don't see a beneficial performance gain for that kind of increase in the complexity. If we do that, then we will have to add more annotations to hide kubernetesGroupId in the json object serialization. Because we do not want users to change the kubernetes groupId when updating host objects. This is un-necessary complexity for me. So I'm - 0 on this. What do others think? Thanks
