Bala.FA has posted comments on this change.

Change subject: gluster: VDSM Gluster verbs to manage UFO/swift configuration
......................................................................


Patch Set 17: I would prefer that you didn't submit this

(5 inline comments)

Sorry.  Some review could have be done much early.

....................................................
File vdsm/gluster/swift.py
Line 43:     elif configOption in items:
Line 44:         return {configOption: items[configOption]}
Line 45:     else:
Line 46:         errMsg = "%s: Invalid Swift Config option" % configOption
Line 47:         raise ge.GlusterSwiftConfigInvalidOptionException(err=[errMsg])
Can you pass configOption to GlusterSwiftConfigInvalidOptionException() and 
form the error message there?
Line 48: 
Line 49: 
Line 50: def _openSwiftConfigFile(serverType):
Line 51:     config = ConfigParser()


Line 50: def _openSwiftConfigFile(serverType):
Line 51:     config = ConfigParser()
Line 52:     if serverType not in SWIFT_CONFIG_FILES:
Line 53:         errMsg = "%s: Invalid Swift Server type" % serverType
Line 54:         raise ge.GlusterSwiftConfigInvalidServerException(err=[errMsg])
same here
Line 55: 
Line 56:     config_file = SWIFT_CONFIG_FILES[serverType]
Line 57:     try:
Line 58:         config.readfp(open(config_file))


Line 57:     try:
Line 58:         config.readfp(open(config_file))
Line 59:     except IOError as e:
Line 60:         errMsg = "[Errno %s] %s: '%s'" % (e.errno, e.strerror, 
e.filename)
Line 61:         raise ge.GlusterSwiftConfigOpenFailedException(err=[errMsg])
How about err=unicode(e)?
Line 62: 
Line 63:     return config
Line 64: 
Line 65: 


Line 78:         # 
mail.python.org/pipermail/python-list/2005-February/342893.html
Line 79:         shutil.move(tempConfigFile.name, config_file)
Line 80:     except IOError as e:
Line 81:         errMsg = "[Errno %s] %s: '%s'" % (e.errno, e.strerror, 
e.filename)
Line 82:         raise ge.GlusterSwiftConfigWriteFailedException(err=[errMsg])
same here
Line 83: 
Line 84: 
Line 85: def _configGet(serverType, section=None, configOption=None):
Line 86:     """


Line 111:                                                       configOption)
Line 112:     elif section:
Line 113:         if not config.has_section(section):
Line 114:             errMsg = "%s: Invalid Swift Config Section" % section
Line 115:             raise 
ge.GlusterSwiftConfigInvalidSectionException(err=[errMsg])
construct error message inside the exception
Line 116: 
Line 117:         items = dict(config.items(section))
Line 118:         configValues[section] = _getConfigOptions(items, configOption)
Line 119:     else:


--
To view, visit http://gerrit.ovirt.org/10864
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie966fb515275a0768f67cbbe2055a07002355327
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Aravinda VK <avish...@redhat.com>
Gerrit-Reviewer: Aravinda VK <avish...@redhat.com>
Gerrit-Reviewer: Bala.FA <barum...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com>
Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to