> On Aug. 13, 2014, 8:13 a.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.
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. - Santhosh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24544/#review50430 ----------------------------------------------------------- On Aug. 11, 2014, 10:20 a.m., Gaurav Aradhye wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24544/ > ----------------------------------------------------------- > > (Updated Aug. 11, 2014, 10:20 a.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 > >
