Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Content-Type: multipart/alternative;
 boundary="===============3970346242304626982=="
MIME-Version: 1.0
Subject: Re: Review Request 23197: CLOUDSTACK-7031 Improve error handling in
 deployDataCenter.py
From: "Santhosh Edukulla" <santhosh.edukulla@citrix.com>
To: "SrikanteswaraRao Talluri" <srikanteswararao.talluri@citrix.com>
Cc: "Santhosh Edukulla" <santhosh.edukulla@citrix.com>,
 "Alex Brett" <alex.brett@citrix.com>,
 "cloudstack" <dev@cloudstack.apache.org>
Date: Tue, 01 Jul 2014 14:52:41 -0000
Message-ID: <20140701145241.5196.23018@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Santhosh Edukulla" <noreply@reviews.apache.org>
References: <20140701143446.5196.76866@reviews.apache.org>
In-Reply-To: <20140701143446.5196.76866@reviews.apache.org>
Reply-To: "Santhosh Edukulla" <santhosh.edukulla@citrix.com>

--===============3970346242304626982==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit


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


tools/marvin/marvin/deployDataCenter.py
<https://reviews.apache.org/r/23197/#comment82667>

    Move this to inside cloudstackException module.


tools/marvin/marvin/deployDataCenter.py
<https://reviews.apache.org/r/23197/#comment82664>

    print statements provide useful info to the people on the console, when they run it from shell and people use it as standalone script to deploy on various occasions. We can do one thing, use a local log function, pass the msg to it, there  dump the message  passed to both streams i.e., stdout and file and use that log function. Or enhance current logging module to provide facilitation where it can take an argument to dump to both stdout and file stream, default set to false.


tools/marvin/marvin/deployDataCenter.py
<https://reviews.apache.org/r/23197/#comment82668>

    As well , please add me as reviewer


tools/marvin/marvin/deployDataCenter.py
<https://reviews.apache.org/r/23197/#comment82665>

    Why we need both exception and debug message, is it not confusing both to read here as well in logs?


tools/marvin/marvin/deployDataCenter.py
<https://reviews.apache.org/r/23197/#comment82666>

    If we raise exception here, will cleanup be called if some thing fails in between?


- Santhosh Edukulla


On July 1, 2014, 2:34 p.m., Alex Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23197/
> -----------------------------------------------------------
> 
> (Updated July 1, 2014, 2:34 p.m.)
> 
> 
> Review request for cloudstack and SrikanteswaraRao Talluri.
> 
> 
> Bugs: CLOUDSTACK-7031
>     https://issues.apache.org/jira/browse/CLOUDSTACK-7031
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> The DeployDataCenters class should not call sys.exit directly, instead we now raise an exception, which can be handled as required.
> 
> In addition this patch tidies up logging by removing calls to print from inside the class, and ensures after a successful deploy / remove, we return an exit code of 0, rather than the previous behaviour that always returned an exit code of 1.
> 
> 
> Diffs
> -----
> 
>   tools/marvin/marvin/deployDataCenter.py c097238 
> 
> Diff: https://reviews.apache.org/r/23197/diff/
> 
> 
> Testing
> -------
> 
> Run through pylint, manual testing now in progress to verify functionality...
> 
> 
> Thanks,
> 
> Alex Brett
> 
>


--===============3970346242304626982==--