cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gaurav Aradhye" <gaurav.arad...@clogeny.com>
Subject Re: Review Request 24544: CLOUDSTACK-7294: Passing listall =True for listUsers api admin call
Date Thu, 21 Aug 2014 06:10:17 GMT


> On Aug. 13, 2014, 1:43 p.m., Santhosh Edukulla wrote:
> > tools/marvin/marvin/cloudstackTestClient.py, line 348
> > <https://reviews.apache.org/r/24544/diff/1/?file=657577#file657577line348>
> >
> >     Make listall as keyword argument, instead of always making it to set to true.
> 
> Gaurav Aradhye wrote:
>     Are you suggesting to add it as a parameter to the function? If I add it as keyword
argument here, it won't make much sense because the main task of function is creating the
api client. listUsers is one of the operations we do inside the function.
> 
> Santhosh Edukulla wrote:
>     Hardcoding inside to listall=True is not the right approach, instead createuserapiclient
is called from getUserApiClient, so here we can make it keyword argument with listall=True,
and pass this arg to createuserapiclient. That we we can control the behavior of what we wanted
and default set to true using get function. Down the lane if behavior changed, just like we
added here inside the code, then instead of that we will control from outside layer with out
touching its internal.

This was already committed, I will add a new patch for incorporating your comments. Closing
this review request.


- Gaurav


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


On Aug. 11, 2014, 3:50 p.m., Gaurav Aradhye wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24544/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2014, 3:50 p.m.)
> 
> 
> Review request for cloudstack, Doug Clark and Santhosh Edukulla.
> 
> 
> Bugs: CLOUDSTACK-7294
>     https://issues.apache.org/jira/browse/CLOUDSTACK-7294
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> According to API changes (https://cwiki.apache.org/confluence/display/CLOUDSTACK/List*+API+commands+rules),
listall parameter is necessary when admin wants to list the resources which belong to some
other account.
> 
> Adding domainid and listall parameter while making listUsers api call through admin.
> 
> 
> Diffs
> -----
> 
>   tools/marvin/marvin/cloudstackTestClient.py 1ca79ee 
> 
> Diff: https://reviews.apache.org/r/24544/diff/
> 
> 
> Testing
> -------
> 
> Yes.
> 
> Tested two failing test cases with this change.
> 
> Test Deploy VM with 4 core CPU & verify the usage ... === TestName: test_01_multiple_core_vm_start_stop_instance
| Status : SUCCESS ===
> ok
> 
> ----------------------------------------------------------------------
> Ran 1 test in 627.195s
> 
> OK
> 
> 
> @summary: Test List IP Addresses pagination ... === TestName: test_01_list_ipaddresses_pagination
| Status : SUCCESS ===
> ok
> 
> ----------------------------------------------------------------------
> Ran 1 test in 32.022s
> 
> OK
> 
> 
> Thanks,
> 
> Gaurav Aradhye
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message