Interdiff:
diff --git a/qa/qa_utils.py b/qa/qa_utils.py
index b8ef546..c1a6d22 100644
--- a/qa/qa_utils.py
+++ b/qa/qa_utils.py
@@ -206,11 +206,12 @@ def AssertCommand(cmd, fail=False, node=None,
log_cmd=True, max_seconds=None):
rcode = popen.returncode
duration_seconds = TimedeltaToTotalSeconds(datetime.datetime.now() -
start)
- if fail is not None:
- _AssertRetCode(rcode, fail, cmdstr, nodename)
-
- if log_cmd:
- _PrintCommandOutput(stdout, stderr)
+ try:
+ if fail is not None:
+ _AssertRetCode(rcode, fail, cmdstr, nodename)
+ finally:
+ if log_cmd:
+ _PrintCommandOutput(stdout, stderr)
if max_seconds is not None:
if duration_seconds > max_seconds:
On Wed, Mar 18, 2015 at 5:15 PM, Hrvoje Ribicic <[email protected]> wrote:
> On Wed, Mar 18, 2015 at 5:10 PM, Klaus Aehlig <[email protected]> wrote:
>
>> > def AssertCommand(cmd, fail=False, node=None, log_cmd=True,
>> max_seconds=None):
>> > """Checks that a remote command succeeds.
>> >
>> > @@ -186,12 +205,12 @@ def AssertCommand(cmd, fail=False, node=None,
>> log_cmd=True, max_seconds=None):
>> > stdout, stderr = popen.communicate()
>> > rcode = popen.returncode
>> > duration_seconds = TimedeltaToTotalSeconds(datetime.datetime.now() -
>> start)
>> > +
>> > if fail is not None:
>> > - try:
>> > - _AssertRetCode(rcode, fail, cmdstr, nodename)
>> > - except:
>> > - print "Stdout was:\n%s\nStderr was:\n%s\n" % (stdout, stderr)
>> > - raise
>> > + _AssertRetCode(rcode, fail, cmdstr, nodename)
>> > +
>> > + if log_cmd:
>> > + _PrintCommandOutput(stdout, stderr)
>>
>> I'm a bit confused by the logic of this change. The old logic was to not
>> log,
>> but in case of an assertion failure do (therefore the try ... except
>> <log> raise).
>>
>> Now it seems we're logging only if no error occured. Note that the
>> _AssertRetCode will through an exception in case of failre and the
>> if log_cmd will never be executed.
>>
>> Maybe the "if log_cmd ..." should go in a finally block?
>>
>
> Indeed it should. My bad, I will send an interdiff.
>
>
>>
>> --
>> Klaus Aehlig
>> Google Germany GmbH, Dienerstr. 12, 80331 Muenchen
>> Registergericht und -nummer: Hamburg, HRB 86891
>> Sitz der Gesellschaft: Hamburg
>> Geschaeftsfuehrer: Graham Law, Christine Elizabeth Flores
>>
>
> Hrvoje Ribicic
> Ganeti Engineering
> Google Germany GmbH
> Dienerstr. 12, 80331, München
>
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
> Geschäftsführer: Graham Law, Christine Elizabeth Flores
> Steuernummer: 48/725/00206
> Umsatzsteueridentifikationsnummer: DE813741370
>
Hrvoje Ribicic
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370
