> On May 8, 2014, 5:06 p.m., Santhosh Edukulla wrote: > > tools/marvin/marvin/lib/common.py, line 1103 > > <https://reviews.apache.org/r/20316/diff/2/?file=567838#file567838line1103> > > > > Please dont add asserts to lib functions, just check the output in test > > code and take it accordingly.
Assert is inside try catch block, so exception won't be raised directly from library. We are returning FAIL in case of exception, along with the exception message. > On May 8, 2014, 5:06 p.m., Santhosh Edukulla wrote: > > tools/marvin/marvin/lib/common.py, line 1122 > > <https://reviews.apache.org/r/20316/diff/2/?file=567838#file567838line1122> > > > > Please remove asserts from libs. It is inside try catch block. > On May 8, 2014, 5:06 p.m., Santhosh Edukulla wrote: > > tools/marvin/marvin/lib/common.py, line 1203 > > <https://reviews.apache.org/r/20316/diff/2/?file=567838#file567838line1203> > > > > This as well can be added to base.py. Maintaining too much common > > functions dependent upon base.py libraries is current overhead, > > VirtualMachine_obj.getState(args) can give us is expunged or stopped or > > started etc. When VM is expunged, we do not get its state as "Expunged". List API throws exception in this case as it does not find the VM id. Hence this case is not a candidate for getState function in base.py > On May 8, 2014, 5:06 p.m., Santhosh Edukulla wrote: > > tools/marvin/marvin/lib/common.py, line 1185 > > <https://reviews.apache.org/r/20316/diff/2/?file=567838#file567838line1185> > > > > Can we move this function inside Snapshot class only? Moved inside snapshot class > On May 8, 2014, 5:06 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_ps_domain_limits.py, line 51 > > <https://reviews.apache.org/r/20316/diff/2/?file=567831#file567831line51> > > > > Also, we can do a quick code walkthrough to understand the flow. It > > will help to do it more thoroughly. We can do a gtm. Sure - Gaurav ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20316/#review42477 ----------------------------------------------------------- On April 24, 2014, 11:01 p.m., Gaurav Aradhye wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20316/ > ----------------------------------------------------------- > > (Updated April 24, 2014, 11:01 p.m.) > > > Review request for cloudstack, Girish Shilamkar and Santhosh Edukulla. > > > Bugs: CLOUDSTACK-1466 > https://issues.apache.org/jira/browse/CLOUDSTACK-1466 > > > Repository: cloudstack-git > > > Description > ------- > > Adding test suits in Primary Storage Limits test cases. > 1)Root/Domain admin limits > 2)Domain Limits > 3)Resize volume > 4)Project Limits > 5)Maximum Limits > > > Diffs > ----- > > test/integration/component/test_ps_domain_limits.py PRE-CREATION > test/integration/component/test_ps_limits.py PRE-CREATION > test/integration/component/test_ps_max_limits.py PRE-CREATION > test/integration/component/test_ps_project_limits.py PRE-CREATION > test/integration/component/test_ps_resize_volume.py PRE-CREATION > tools/marvin/marvin/codes.py 4d44c58 > tools/marvin/marvin/lib/base.py d753098 > tools/marvin/marvin/lib/common.py 8868d2d > > Diff: https://reviews.apache.org/r/20316/diff/ > > > Testing > ------- > > Yes > > > Thanks, > > Gaurav Aradhye > >
