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
