Change in vdsm[master]: Move multipath configuration to vdsm-tool configurator
danken at redhat.com
danken at redhat.com
Wed Mar 26 18:56:00 UTC 2014
Dan Kenigsberg has posted comments on this change.
Change subject: Move multipath configuration to vdsm-tool configurator
......................................................................
Patch Set 2: Code-Review-1
(3 comments)
Very very partial review.
Note the persistent pep8 errors
lib/vdsm/tool/configurator.py:217:26: E128 continuation line under-indented for visual indent
lib/vdsm/tool/configurator.py:220:26: E124 closing bracket does not match visual indentation
lib/vdsm/tool/configurator.py:267:5: E303 too many blank lines (2)
http://gerrit.ovirt.org/#/c/26123/2/lib/vdsm/utils.py
File lib/vdsm/utils.py:
Line 154: else:
Line 155: raise
Line 156:
Line 157:
Line 158: def rotateFiles(directory, prefixName, gen, cp=False, persist=False):
this function is so old and dusty. would you break this big patch to smaller ones, and have a separate patch re-implementing this function?
I, for one, do not recall why we care about cp=True. I certainly resent the silent swallowing of exceptions. Luckily, when used by an external vdsm-tool, there is no excuse for doing that.
Line 159: logging.debug("dir: %s, prefixName: %s, versions: %s" %
Line 160: (directory, prefixName, gen))
Line 161: gen = int(gen)
Line 162: files = os.listdir(directory)
Line 203: except:
Line 204: pass
Line 205: if isOvirtNode() and persist and not cp:
Line 206: try:
Line 207: execCmd([constants.EXT_PERSIST, newName], logErr=False,
we already have a function wrapper for this (persistFile). Let's use it.
Line 208: sudo=True)
Line 209: except:
Line 210: pass
Line 211:
Line 206: try:
Line 207: execCmd([constants.EXT_PERSIST, newName], logErr=False,
Line 208: sudo=True)
Line 209: except:
Line 210: pass
omg
Line 211:
Line 212:
Line 213: IPXMLRPCRequestHandler = SimpleXMLRPCRequestHandler
Line 214:
--
To view, visit http://gerrit.ovirt.org/26123
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I40f802477e39000c5cae01a496ac2d9f879ebfa8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list