Dan Kenigsberg has posted comments on this change.
Change subject: glusterHostsList verb uses gluster cli xml output.
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File vdsm/gluster/cli.py
Line 541: return hostList
Line 542:
Line 543:
Line 544: @exportToSuperVdsm
Line 545: def peerStatus():
hmmm, it is not very nice. this verb assume that the client and server sits on
the same host. I suppose it would make more sense for this verb to receive the
address of the server, and use _getLocalIpAddress() or _getGlusterHostName()
only as a default.
Line 546: """
Line 547: Returns:
Line 548: [{'hostname': HOSTNAME, 'uuid': UUID, 'status': STATE}, ...]
Line 549: """
Line 553: except ge.GlusterCmdFailedException, e:
Line 554: raise ge.GlusterHostsListFailedException(rc=e.rc, err=e.err)
Line 555: try:
Line 556: return _parsePeerStatus(xmltree,
Line 557: _getLocalIpAddress() or
_getGlusterHostName(),
would you explain why you prefer getLocalIpAddress on getGlusterHostName? this
change may fit into a separate patch - is it related to the move to xml?
Line 558: _getGlusterUuid(),
HostStatus.CONNECTED)
Line 559: except:
Line 555: try:
Line 556: return _parsePeerStatus(xmltree,
Line 557: _getLocalIpAddress() or
_getGlusterHostName(),
Line 558: _getGlusterUuid(),
HostStatus.CONNECTED)
Line 559: except:
"except" is way too wide to be correct. assume a network error while in
_getGlusterHostName(). or a typo hiding somewhere. you do not want to swallow
everything.
--
To view, visit http://gerrit.ovirt.org/7617
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f96a6ac76fd9ac7ea30fa1c707d61e9f05eb42
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA <[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: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
