Change in vdsm[master]: execCmd: Make loggign when successful optional
nsoffer at redhat.com
nsoffer at redhat.com
Fri Nov 22 18:35:44 UTC 2013
Nir Soffer has posted comments on this change.
Change subject: execCmd: Make loggign when successful optional
......................................................................
Patch Set 2:
(3 comments)
I think this patch can be more useful if it allows to turn off all logging in execCmd. This should be under complete control of the caller.
....................................................
File lib/vdsm/utils.py
Line 489:
Line 490: def execCmd(command, sudo=False, cwd=None, data=None, raw=False, logErr=True,
Line 491: printable=None, env=None, sync=True, nice=None, ioclass=None,
Line 492: ioclassdata=None, setsid=False, execCmdLogger=logging.root,
Line 493: deathSignal=0, logWhenSuccessful=True):
I would call this "verbose"
Line 494: """
Line 495: Executes an external command, optionally via sudo.
Line 496:
Line 497: IMPORTANT NOTE: don't define a deathSignal when sync=False
Line 516: if not printable:
Line 517: printable = command
Line 518:
Line 519: cmdline = repr(subprocess.list2cmdline(printable))
Line 520: execCmdLogger.debug("%s (cwd %s)", cmdline, cwd)
You should disable also this log:
if verbose:
# log command line here
Line 521:
Line 522: p = BetterPopen(command, close_fds=True, cwd=cwd, env=env,
Line 523: deathSignal=deathSignal)
Line 524: p = AsyncProc(p)
Line 534: if out is None:
Line 535: # Prevent splitlines() from barfing later on
Line 536: out = ""
Line 537:
Line 538: if logWhenSuccessful or p.returncode != 0:
You don't need to check error code here. The caller gets all the information needed to log the proper error if needed.
if verbose:
# log debug here
If you want to turn off only return value debug logs, you don't have to add a new argument, there is already a logErr argument. Note that Saggi rejected the last patch that tried to do that though.
Line 539: execCmdLogger.debug(
Line 540: "%s: <err> = %s; <rc> = %d",
Line 541: {True: "SUCCESS", False: "FAILED"}[p.returncode == 0], repr(err),
Line 542: p.returncode)
--
To view, visit http://gerrit.ovirt.org/21515
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c550aded704dd944f4f2af88b7925f9473f1a4a
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list