cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Santhosh Edukulla" <santhosh.eduku...@citrix.com>
Subject Re: Review Request 17789: method to handle default template changes
Date Sat, 08 Feb 2014 13:45:29 GMT

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



tools/marvin/marvin/integration/lib/common.py
<https://reviews.apache.org/r/17789/#comment63941>

    The name is also little confusing. It is basically getting an ostype name for a featured
and builtin template which is in ready state. Modify the name to suite its flow



tools/marvin/marvin/integration/lib/common.py
<https://reviews.apache.org/r/17789/#comment63945>

    use try\except block to wrap and return exception code in case of an exception.



tools/marvin/marvin/integration/lib/common.py
<https://reviews.apache.org/r/17789/#comment63940>

    Add doc strings for this method.



tools/marvin/marvin/integration/lib/common.py
<https://reviews.apache.org/r/17789/#comment63946>

    Also, i believe the need for usage of this function. Why cant it be handled inside of
get_template only. If user provided ostype is not matching return the record which is builtin
featured and ready. Why need a separate function. When people use this and when other? Any
cases?



tools/marvin/marvin/integration/lib/common.py
<https://reviews.apache.org/r/17789/#comment63947>

    If it is tested, which cases have modified, please add them here and along with their
logs?



tools/marvin/marvin/integration/lib/common.py
<https://reviews.apache.org/r/17789/#comment63942>

    Please dont add assert in library functions. Use return codes and assert for your test
case. Assertion for library is not good.



tools/marvin/marvin/integration/lib/common.py
<https://reviews.apache.org/r/17789/#comment63943>

    why raise and return both? Dont raise exception, with this it will become enforcing on
tests to either catch it or make it to work for tests according to their behavior



tools/marvin/marvin/integration/lib/common.py
<https://reviews.apache.org/r/17789/#comment63944>

    Again remove raise. Use return codes.


- Santhosh Edukulla


On Feb. 6, 2014, 2:06 p.m., suresh sadhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17789/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2014, 2:06 p.m.)
> 
> 
> Review request for cloudstack, Santhosh Edukulla and SrikanteswaraRao Talluri.
> 
> 
> Bugs: cloudstack-6042
>     https://issues.apache.org/jira/browse/cloudstack-6042
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Right now default template for xen changed  and test scripts are still point to old template
,due to this deployvm and other scripts are failing and its hard coded in  each script.
> 
> So I wrote a method to  return  built-in OS name dynamically.
> 
> but here we need to add one line of code in each script
> 
>  
> another  solution is :
> 
> we have get_template method and  again this is depend on hardcoded parameter  type ostype.this
is generic method .  ,if modify exiting logic little bit(it won't be generic and specific
to built-in template)then no need to touch other scripts. 
> 
> 
> Please review and share you input 
> 
> 
> Diffs
> -----
> 
>   tools/marvin/marvin/integration/lib/common.py 550de1a 
> 
> Diff: https://reviews.apache.org/r/17789/diff/
> 
> 
> Testing
> -------
> 
> yes
> 
> 
> Thanks,
> 
> suresh sadhu
> 
>


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