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