Mark Wu has uploaded a new change for review.
Change subject: Remove duplicate implementation of tobool ......................................................................
Remove duplicate implementation of tobool
This patch removes the duplicate tobool function from hooking.py, and replace the usage of it with utils.tobool.
Also make hooking.py pep8 clean.
Change-Id: Ie0d382dabc9df68139070e8ac10f84ba1ca3774b Signed-off-by: Mark Wu wudxw@linux.vnet.ibm.com --- M Makefile.am M vdsm/hooking.py M vdsm_hooks/vhostmd/after_vm_destroy.py M vdsm_hooks/vhostmd/before_vm_start.py 4 files changed, 6 insertions(+), 15 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/8446/1
diff --git a/Makefile.am b/Makefile.am index 7548f42..26ce956 100644 --- a/Makefile.am +++ b/Makefile.am @@ -59,6 +59,7 @@ vdsm/gluster/exception.py \ vdsm/gluster/hostname.py \ vdsm/guestIF.py \ + vdsm/hooking.py \ vdsm/hooks.py \ vdsm/libvirtev.py \ vdsm/libvirtvm.py \ diff --git a/vdsm/hooking.py b/vdsm/hooking.py index a324311..714e046 100644 --- a/vdsm/hooking.py +++ b/vdsm/hooking.py @@ -42,21 +42,10 @@ from storage.misc import execCmd execCmd # make pyflakes happy
-def tobool(s): - """Convert the argument into a boolean""" - try: - if s == None: - return False - if type(s) == bool: - return s - if s.lower() == 'true': - return True - return bool(int(s)) - except: - return False
def read_domxml(): return minidom.parseString(file(os.environ['_hook_domxml']).read())
+ def write_domxml(domxml): file(os.environ['_hook_domxml'], 'w').write(domxml.toxml(encoding='utf-8')) diff --git a/vdsm_hooks/vhostmd/after_vm_destroy.py b/vdsm_hooks/vhostmd/after_vm_destroy.py index c129ded..7c505de 100644 --- a/vdsm_hooks/vhostmd/after_vm_destroy.py +++ b/vdsm_hooks/vhostmd/after_vm_destroy.py @@ -21,7 +21,7 @@
import os import subprocess -import hooking +from vdsm import utils from vdsm import vdscli
s = vdscli.connect() @@ -30,6 +30,6 @@ if res['status']['code'] == 0: if not [v for v in res['vmList'] if v.get('vmId') != os.environ.get('vmId') and - hooking.tobool(v.get('custom', {}).get('sap_agent', False))]: + utils.tobool(v.get('custom', {}).get('sap_agent', False))]: subprocess.call(['/usr/bin/sudo', '-n', '/sbin/service', 'vhostmd', 'stop']) diff --git a/vdsm_hooks/vhostmd/before_vm_start.py b/vdsm_hooks/vhostmd/before_vm_start.py index 47d0d0d..7e38a7f 100644 --- a/vdsm_hooks/vhostmd/before_vm_start.py +++ b/vdsm_hooks/vhostmd/before_vm_start.py @@ -22,9 +22,10 @@ import os import subprocess import hooking +from vdsm import utils
-if hooking.tobool(os.environ.get('sap_agent', False)): +if utils.tobool(os.environ.get('sap_agent', False)): domxml = hooking.read_domxml()
subprocess.call(['/usr/bin/sudo', '-n', '/sbin/service', 'vhostmd',
-- To view, visit http://gerrit.ovirt.org/8446 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: Ie0d382dabc9df68139070e8ac10f84ba1ca3774b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: Remove duplicate implementation of tobool ......................................................................
Patch Set 2: I would prefer that you didn't submit this
(1 inline comment)
.................................................... File vdsm/hooking.py Line 40: from xml.dom import minidom Line 41: Line 42: from storage.misc import execCmd Line 43: execCmd # make pyflakes happy Line 44: hooking.py is a kinda-documented interface that out-of-branch hooks may depend upon.
How about adding
from vdsm.utils import tobool
? Line 45: Line 46: def read_domxml(): Line 47: return minidom.parseString(file(os.environ['_hook_domxml']).read()) Line 48:
-- To view, visit http://gerrit.ovirt.org/8446 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie0d382dabc9df68139070e8ac10f84ba1ca3774b Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com
Mark Wu has posted comments on this change.
Change subject: Remove duplicate implementation of tobool ......................................................................
Patch Set 2:
It would cause of pep8 warning of "'tobool' imported but unused". So don't put pep8 check on it or just leave the original code there? Thanks!
-- To view, visit http://gerrit.ovirt.org/8446 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie0d382dabc9df68139070e8ac10f84ba1ca3774b Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com
Zhou Zheng Sheng has posted comments on this change.
Change subject: Remove duplicate implementation of tobool ......................................................................
Patch Set 2: (1 inline comment)
.................................................... File vdsm/hooking.py Line 40: from xml.dom import minidom Line 41: Line 42: from storage.misc import execCmd Line 43: execCmd # make pyflakes happy Line 44: Markwu, it's a pyflakes's warning, we can follow what line 43 does to make pyflakes happy.
from vdsm.utils import tobool tobool # make pyflakes happy Line 45: Line 46: def read_domxml(): Line 47: return minidom.parseString(file(os.environ['_hook_domxml']).read()) Line 48:
-- To view, visit http://gerrit.ovirt.org/8446 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie0d382dabc9df68139070e8ac10f84ba1ca3774b Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Mark Wu has posted comments on this change.
Change subject: Remove duplicate implementation of tobool ......................................................................
Patch Set 2:
yes, I thought about it. But I don't like it. Any other way to suppress the warning?
-- To view, visit http://gerrit.ovirt.org/8446 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie0d382dabc9df68139070e8ac10f84ba1ca3774b Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Zhou Zheng Sheng has posted comments on this change.
Change subject: Remove duplicate implementation of tobool ......................................................................
Patch Set 2:
As far as I know there is no obvious way to suppress the warning. I thin keep hooking.py in the PEP8WHITELIST is good, add a redundant "tobool" line is OK.
In vdsm/storage/misc.py there is a parseBool function, I think it's the same as utils.tobool. How about remove parseBool to use tobool?
-- To view, visit http://gerrit.ovirt.org/8446 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie0d382dabc9df68139070e8ac10f84ba1ca3774b Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Ryan Harper has posted comments on this change.
Change subject: Remove duplicate implementation of tobool ......................................................................
Patch Set 2: (1 inline comment)
.................................................... File vdsm/hooking.py Line 45: Line 46: def read_domxml(): Line 47: return minidom.parseString(file(os.environ['_hook_domxml']).read()) Line 48: Line 49: I'll assume this is a PEP8 change; should be a separate patch. Line 50: def write_domxml(domxml):
-- To view, visit http://gerrit.ovirt.org/8446 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie0d382dabc9df68139070e8ac10f84ba1ca3774b Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: Remove duplicate implementation of tobool ......................................................................
Patch Set 3: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/8446 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie0d382dabc9df68139070e8ac10f84ba1ca3774b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: Remove duplicate implementation of tobool ......................................................................
Patch Set 3: Verified; Looks good to me, approved
yeah, sometime I verify things by myself.
-- To view, visit http://gerrit.ovirt.org/8446 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ie0d382dabc9df68139070e8ac10f84ba1ca3774b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Shahar Havivi shavivi@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Remove duplicate implementation of tobool ......................................................................
Remove duplicate implementation of tobool
This patch removes the duplicate tobool function from hooking.py, and replace it with utils.tobool.
Change-Id: Ie0d382dabc9df68139070e8ac10f84ba1ca3774b Signed-off-by: Mark Wu wudxw@linux.vnet.ibm.com --- M vdsm/hooking.py 1 file changed, 4 insertions(+), 14 deletions(-)
Approvals: Dan Kenigsberg: Verified; Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/8446 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: Ie0d382dabc9df68139070e8ac10f84ba1ca3774b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Shahar Havivi shavivi@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
vdsm-patches@lists.fedorahosted.org