Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Content-Type: multipart/alternative;
 boundary="===============4992622147358542895=="
MIME-Version: 1.0
Subject: Re: Review Request 19022: List VM enhancement to support querying
 with
 multiple VM IDs
From: "Koushik Das" <koushik.das@citrix.com>
To: "daan Hoogland" <daan.hoogland@gmail.com>,
 "Min Chen" <min.chen@citrix.com>
Cc: "Koushik Das" <koushik.das@citrix.com>,
 "cloudstack" <dev@cloudstack.apache.org>
Date: Tue, 11 Mar 2014 08:34:32 -0000
Message-ID: <20140311083432.13499.54352@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Koushik Das" <noreply@reviews.apache.org>
References: <20140311074108.13499.12113@reviews.apache.org>
In-Reply-To: <20140311074108.13499.12113@reviews.apache.org>
Reply-To: "Koushik Das" <koushik.das@citrix.com>

--===============4992622147358542895==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit


> On March 11, 2014, 7:41 a.m., daan Hoogland wrote:
> > server/src/com/cloud/api/query/QueryManagerImpl.java, line 731
> > <https://reviews.apache.org/r/19022/diff/1/?file=516059#file516059line731>
> >
> >     Why not be lenient and consider the id as part of ids?
> >     You are being strict on something that isn't hurtful but an empty ids when no id is given is not checked!

I haven't removed 'id' due to back-compat. As I have mentioned 'id' and 'ids' are mutually exclusive. Also it is a valid scenario to not pass any of id or ids.


- Koushik


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


On March 11, 2014, 6:29 a.m., Koushik Das wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19022/
> -----------------------------------------------------------
> 
> (Updated March 11, 2014, 6:29 a.m.)
> 
> 
> Review request for cloudstack, daan Hoogland and Min Chen.
> 
> 
> Bugs: CLOUDSTACK-6052
>     https://issues.apache.org/jira/browse/CLOUDSTACK-6052
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Currently list VM can only be called using a single VM ID. So if there is a need to query a set of VMs using ID then either multiple list VM calls need to be made or all VMs needs to be fetched and then do a client side filtering. Both approaches are sub-optimal - the former results in multiple queries to database and the latter will be an overkill if you need a small subset from a very large number of VMs. 
> 
> The proposal is to have an additional parameter to specify a list of VM IDs for which the data needs to be fetched. Using this the required VMs can be queried in an efficient manner. With the new parameter the syntax would look like 
> http://localhost:8096/api?command=listVirtualMachines&listAll=true&ids=eddac053-9b12-4d2e-acb7-233de2e98112,009966fc-4d7b-4f84-8609-254979ba0134 
> 
> The new 'ids' parameter will be mutually exclusive with the existing 'id' parameter.
> 
> 
> Diffs
> -----
> 
>   api/src/org/apache/cloudstack/api/command/user/vm/ListVMsCmd.java 1a564f6 
>   server/src/com/cloud/api/query/QueryManagerImpl.java 4200799 
>   test/integration/smoke/test_deploy_vm.py 425aeb7 
> 
> Diff: https://reviews.apache.org/r/19022/diff/
> 
> 
> Testing
> -------
> 
> Added integration test, also verified manually.
> 
> 
> Thanks,
> 
> Koushik Das
> 
>


--===============4992622147358542895==--