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.
_______________________________________________
Dev mailing list
[email protected]
http://wso2.org/cgi-bin/mailman/listinfo/dev

Reply via email to