>  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?

-- 
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

Reply via email to