Hi Akila, Thank you for the feedback. tenantId is used because it is needed in the url which is used to authenticate. WIthout the tenantId the url would be incomplete. I have fixed [1] all the issues you have mentioned in the previous email except for the Netflix Feign standard and I'm looking into it. About the last point, is it localMemberHostPort or localMemberPort ?
[1] https://github.com/osuran/azure-membership-scheme thanks, On Thu, Jun 30, 2016 at 11:22 AM, Akila Ravihansa Perera <raviha...@wso2.com > wrote: > Hi Osura, > > Good progress :) > > Netflix Feign [1] is a highly customizable and easy to use REST API client > framework. The GitHub page itself contains many samples. We have written > Mesos membership scheme [2] using that which you can use as a reference > guideline. Please refactor your code to use this client. Let me know if you > run into any issues. > > I noticed few more issues in the code; > - No need to define a variable and assign a const [3]. You can directly > use the const. > - Why do you need to append tenantId to authorization endpoint? Just > asking to clarify why tenantId is needed here. > > - You can move common operations to common method. In [4], instead of > checking for empty values and in [5] reading from System.env. Move all that > to getParameterValue method. This will reduce the amount of code you have > to write. > > - You don't have to log and throw [6]. This will only clutter the log > file. > > - You don't need a for loop here [7]. Simply use a for-each iterator. > > - Don't put ":" in front of the placeholder [8] > > - Don't put spaces around square brackets [9] > > - You need to separate variables by commas [10] > > - Use proper case for log msg labels [11]. Should be "VM-IP" or simply > "IP Address" is enough. This should contain both IP and local port. Read > below for more info on local port. > > * This is very important * > - Use localMemberHostPort defined in axis2.xml when adding the member IPs > in [12]. This is the port that all WSO2 server instances are listening on > for Hazelcast connections. By default it is 4000 that's why it works now. > But for some reason if someone changes it, then this code will break. > > [1] https://github.com/Netflix/feign > [2] > https://github.com/wso2/mesos-artifacts/tree/master/common/mesos-membership-scheme > [3] > https://github.com/osuran/azure-membership-scheme/blob/master/src/main/java/org/wso2/carbon/clustering/azure/AzureMembershipScheme.java#L101 > [4] > https://github.com/osuran/azure-membership-scheme/blob/master/src/main/java/org/wso2/carbon/clustering/azure/AzureMembershipScheme.java#L113 > [5] > https://github.com/osuran/azure-membership-scheme/blob/master/src/main/java/org/wso2/carbon/clustering/azure/AzureMembershipScheme.java#L110 > [6] > https://github.com/osuran/azure-membership-scheme/blob/master/src/main/java/org/wso2/carbon/clustering/azure/AzureMembershipScheme.java#L182 > [7] > https://github.com/osuran/azure-membership-scheme/blob/master/src/main/java/org/wso2/carbon/clustering/azure/AzureMembershipScheme.java#L200 > [8] > https://github.com/osuran/azure-membership-scheme/blob/master/src/main/java/org/wso2/carbon/clustering/azure/AzureMembershipScheme.java#L263 > [9] > https://github.com/osuran/azure-membership-scheme/blob/master/src/main/java/org/wso2/carbon/clustering/azure/AzureMembershipScheme.java#L282 > [10] > https://github.com/osuran/azure-membership-scheme/blob/master/src/main/java/org/wso2/carbon/clustering/azure/AzureMembershipScheme.java#L171 > [11] > https://github.com/osuran/azure-membership-scheme/blob/master/src/main/java/org/wso2/carbon/clustering/azure/AzureMembershipScheme.java#L177 > [12] > https://github.com/osuran/azure-membership-scheme/blob/master/src/main/java/org/wso2/carbon/clustering/azure/AzureMembershipScheme.java#L176 > > Thanks. > > -- Regards, Osura Rathnayake
_______________________________________________ Dev mailing list Dev@wso2.org http://wso2.org/cgi-bin/mailman/listinfo/dev
