Federico Simoncelli has posted comments on this change. Change subject: gluster: Added gluster hooks support ......................................................................
Patch Set 22: (2 inline comments) Few comments about the exceptions. .................................................... File vdsm/gluster/exception.py Line 397: code = 4500 Line 398: message = "Gluster Hook Exception" Line 399: Line 400: Line 401: class GlusterHookListException(GlusterException): GlusterHookListException, GlusterHookEnableFailedException and GlusterHookDisableFailedException aren't giving any additional information about what happened and the command is known by then client. I'd just use GlusterHookException. If you need a nice error you can do it on the client side. Line 402: code = 4501 Line 403: message = "List gluster hook failed" Line 404: Line 405: Line 412: code = 4503 Line 413: message = "Disable gluster hook failed" Line 414: Line 415: Line 416: class GlusterHookAlreadyEnabledException(GlusterHookException): Both GlusterHookAlreadyEnabledException and GlusterHookAlreadyDisabledException look redundant. If you are trying to enable an hook that is already enabled (or disable one that is disabled) there's no need to fail, you can just return success. Line 417: code = 4504 Line 418: Line 419: def __init__(self, glusterCmd=None, level=None, hookName=None): Line 420: self.glusterCmd = glusterCmd -- To view, visit http://gerrit.ovirt.org/9671 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3918aa035d90967f1297dc7fadcf14b6a9385c45 Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Timothy Asir <[email protected]> Gerrit-Reviewer: Aravinda VK <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Bala.FA <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Timothy Asir <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
