Re: Review Request 23805: Added "state" parameter to the "listPublicIpAddresses" API call

[Rohit Yadav <bh...@apache.org>]

> On July 29, 2014, 10:20 a.m., Rohit Yadav wrote:
> > Looks fine, we still need to test it before we land it on master. I unfortunately don't have my hardware (and some parts) setup will try with DevCloud or hope someone else would review, test and ship it :)
> 
> Ilia Shakitko wrote:
>     Maybe we can add a reviewer who is currently working on master branch?

Hi, I was going to apply it today but your patch is not a git-formatted patch. Please upload a new git formatted patch, ref: https://cwiki.apache.org/confluence/display/CLOUDSTACK/Git#Git-Patches


- Rohit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23805/#review48976
-----------------------------------------------------------


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
> 
>