Dan Kenigsberg has posted comments on this change. Change subject: Added _execGlusterXml() ......................................................................
Patch Set 6: I would prefer that you didn't submit this (3 inline comments) .................................................... File vdsm/gluster/cli.py Line 72: rc, out, err = utils.execCmd(cmd) Line 73: if rc != 0: Line 74: raise ge.GlusterCmdExecFailedException(rc, out, err) Line 75: try: Line 76: tree = etree.fromstring('\n'.join(out)) you'd save some time if you call execCmd(raw=True) Line 77: rv = tree.find('opRet').text Line 78: msg = tree.find('opErrstr').text Line 79: except (etree.ParseError, AttributeError): Line 80: raise ge.GlusterXmlErrorException(err=out) Line 78: msg = tree.find('opErrstr').text Line 79: except (etree.ParseError, AttributeError): Line 80: raise ge.GlusterXmlErrorException(err=out) Line 81: if rv == '0': Line 82: return (tree, out) 'tree' is 'out' parsed. so why do you need to return both? Line 83: else: Line 84: raise ge.GlusterCmdFailedException(rc=rv, err=[msg]) Line 85: Line 86: Line 80: raise ge.GlusterXmlErrorException(err=out) Line 81: if rv == '0': Line 82: return (tree, out) Line 83: else: Line 84: raise ge.GlusterCmdFailedException(rc=rv, err=[msg]) here rc is set to a string; else where it is an int. that's legal but confusing. Line 85: Line 86: Line 87: def _parseVolumeInfo(out): Line 88: if not out[0].strip(): -- To view, visit http://gerrit.ovirt.org/6999 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I385f992005d446b417be84eb7ff484e4edf6e5b6 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Bala.FA <barum...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@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: Timothy Asir <tjeya...@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