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 <[email protected]>
Gerrit-Reviewer: Assaf Muller <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
