> On July 28, 2014, 12:43 p.m., Rohit Yadav wrote: > > api/src/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java, > > lines 205-207 > > <https://reviews.apache.org/r/23805/diff/1/?file=639247#file639247line205> > > > > This was removed.
I deleted it because it's not being used in the code. There is a second method: "public Boolean isAllocatedOnly()" this one is used instead. > On July 28, 2014, 12:43 p.m., Rohit Yadav wrote: > > api/src/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java, > > lines 156-166 > > <https://reviews.apache.org/r/23805/diff/1/?file=639247#file639247line156> > > > > I guess moving it above was a cosmetic change. Please fix that you > > don't remove/change anything in the move refactor. Is this still the case according to the written comments below? > On July 28, 2014, 12:43 p.m., Rohit Yadav wrote: > > api/src/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java, > > lines 156-167 > > <https://reviews.apache.org/r/23805/diff/1/?file=639247#file639247line156> > > > > I guess the move refactor was a cosmetic change, but it removed > > getAllocatedOnly() method. Please fix it, or explain why we need to remove > > it? Because it's a duplicated method, isn't it? As well is "isForLoadBalancing" and "getForLoadBalancing" (just noticed, I'll check for these methods usages as well later) - Ilia ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23805/#review48842 ----------------------------------------------------------- On July 28, 2014, 10:12 a.m., Ilia Shakitko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23805/ > ----------------------------------------------------------- > > (Updated July 28, 2014, 10:12 a.m.) > > > Review request for cloudstack, Alena Prokharchyk, Alex Huang, Harikrishna > Patnala, Kishan Kavala, Prachi Damle, Rohit Yadav, Ilia Shakitko, Wei Zhou, > and Wido den Hollander. > > > Bugs: CLOUDSTACK-7159 > https://issues.apache.org/jira/browse/CLOUDSTACK-7159 > > > Repository: cloudstack-git > > > Description > ------- > > This improvement is introducing a new parameter for the > "listPublicIpAddresses" API call - "state". > > Few times we've faced an impedemence of having a list of "Free" > publicIpAddresses. You have to go thru all the IPs with (allocatedonly = > false) and filter out the "Free" once. It's not a big deal, but it's an extra > time and traffic between CloudStack and an API consumer. > > I also moved few methods out of the 'API Implementation' and put them above > as a minor refactoring. > Method "getForLoadBalancing" has been removed because it's not being used in > code anywhere else. > > This patch is done for "master" branch. > > > Diffs > ----- > > > api/src/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java > 07ccfe9 > server/src/com/cloud/server/ManagementServerImpl.java 99b12732 > > Diff: https://reviews.apache.org/r/23805/diff/ > > > Testing > ------- > > 1) Build successfull > 2) No tests broken > 3) Tested few different calls with cloudmonkey: > > list publicipaddresses forvirtualnetwork=false listall=true page=1 pagesize=0 > count = 10 > > list publicipaddresses forvirtualnetwork=false listall=true > allocatedonly=false page=1 pagesize=0 > count = 100 > > list publicipaddresses forvirtualnetwork=false listall=true state=free page=1 > pagesize=0 > count = 90 > > > Thanks, > > Ilia Shakitko > >
