Discussion:
Change in vdsm[master]: tool.service: avoid execCmd(raw=False)
Code Review
2017-08-05 12:51:22 UTC
Permalink
From Dan Kenigsberg <***@redhat.com>:

Dan Kenigsberg has uploaded a new change for review.

Change subject: tool.service: avoid execCmd(raw=False)
......................................................................

tool.service: avoid execCmd(raw=False)

tool.service is the only place where we use raw=False to split lines in
command output. However, we concatenate these lines immediately
afterwards, or ignore them.

This patch would enable the removal of the `raw` argument from execCmd
in a future patch.

Change-Id: I7530451496c41d1f3568e263f1087a92a8f0f3bb
Signed-off-by: Dan Kenigsberg <***@redhat.com>
---
M lib/vdsm/tool/service.py
1 file changed, 4 insertions(+), 8 deletions(-)


git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/13/80213/1

diff --git a/lib/vdsm/tool/service.py b/lib/vdsm/tool/service.py
index b720b4a..f7fd9e7 100644
--- a/lib/vdsm/tool/service.py
+++ b/lib/vdsm/tool/service.py
@@ -30,11 +30,7 @@

from vdsm.common.cmdutils import CommandPath
from . import expose, UsageError, ExtraArgsError
-from ..commands import execCmd as _execCmd
-
-
-def execCmd(argv, raw=True, *args, **kwargs):
- return _execCmd(argv, raw=raw, *args, **kwargs)
+from ..commands import execCmd


_SYSTEMCTL = CommandPath("systemctl",
@@ -108,10 +104,10 @@
@functools.wraps(systemctlFun)
def wrapper(srvName):
cmd = [_SYSTEMCTL.cmd, "--no-pager", "list-unit-files"]
- rc, out, err = execCmd(cmd, raw=False)
+ rc, out, err = execCmd(cmd)
if rc != 0:
raise ServiceOperationError(
- "Error listing unit files", '\n'.join(out), '\n'.join(err))
+ "Error listing unit files", out, err)
fullName = srvName
# If unit file type was specified, don't override it.
if srvName.count('.') < 1:
@@ -179,7 +175,7 @@
@functools.wraps(initctlFun)
def wrapper(srvName):
cmd = [_INITCTL.cmd, "usage", srvName]
- rc, out, err = execCmd(cmd, raw=False)
+ rc, out, err = execCmd(cmd)
if rc != 0:
raise ServiceNotExistError("%s is not an Upstart service" %
srvName)


--
To view, visit https://gerrit.ovirt.org/80213
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7530451496c41d1f3568e263f1087a92a8f0f3bb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <***@redhat.com>
_______________________________________________
vdsm-patches mailing list -- vdsm-***@lists.fedorahosted.org
To unsubscribe send an email to
Code Review
2017-08-19 19:35:53 UTC
Permalink
From Dan Kenigsberg <***@redhat.com>:

Dan Kenigsberg has posted comments on this change.

Change subject: tool.service: avoid execCmd(raw=False)
......................................................................


Patch Set 2: Verified+1

--
To view, visit https://gerrit.ovirt.org/80213
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7530451496c41d1f3568e263f1087a92a8f0f3bb
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <***@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <***@redhat.com>
Gerrit-Reviewer: Irit Goihman <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <***@gmail.com>
Gerrit-Reviewer: gerrit-hooks <***@ovirt.org>
Gerrit-HasComments: No
_______________________________________________
vdsm-patches mailing list -- vdsm-***@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-***@lists.fedorah
Code Review
2017-08-19 20:10:39 UTC
Permalink
From Dan Kenigsberg <***@redhat.com>:

Dan Kenigsberg has submitted this change and it was merged. ( https://gerrit.ovirt.org/80213 )

Change subject: tool.service: avoid execCmd(raw=False)
......................................................................


tool.service: avoid execCmd(raw=False)

tool.service.execCmd has raw=True as its default,but two places we use
raw=False to split lines in command output. However, we concatenate
these lines immediately afterwards, or ignore them.

Change-Id: I7530451496c41d1f3568e263f1087a92a8f0f3bb
Signed-off-by: Dan Kenigsberg <***@redhat.com>
---
M lib/vdsm/tool/service.py
1 file changed, 3 insertions(+), 3 deletions(-)

Approvals:
Piotr Kliczewski: Looks good to me, approved
Jenkins CI: Passed CI tests
Dan Kenigsberg: Verified



--
To view, visit https://gerrit.ovirt.org/80213
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I7530451496c41d1f3568e263f1087a92a8f0f3bb
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <***@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <***@redhat.com>
Gerrit-Reviewer: Irit Goihman <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <***@gmail.com>
Gerrit-Reviewer: gerrit-hooks <***@ovirt.org>
_______________________________________________
vdsm-patches mailing list -- vdsm-***@lists.fedorahosted.org
To unsubscribe send an email to v

Loading...