Federico Simoncelli has posted comments on this change.
Change subject: Added glusterVolumeStatus verb
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(6 inline comments)
....................................................
File vdsm_cli/vdsClientGluster.py
Line 198: if len(args) == 2:
No need for this, you could use:
self.s.glusterVolumeStatus(*args)
Even though I don't like much this approach, I'd like to see the arguments
(with a meaningful name). These comments applies to all the other methods in
this file.
....................................................
File vdsm/gluster/cli.py
Line 89: status['name'] = tree.find('volStatus/volName').text
What about:
status = {
'name': tree.find('volStatus/volName').text
'bricks': []
'nfs': []
'shd': []
}
And later on:
status['nfs'].append(...
status['bricks'].append(...
status['shd'].append(...
return status
Line 95: value = {}
Blank line.
Line 103: value['status'] = 'OFFLINE'
Let's add a blank line here to separate. We're going to start another logical
block.
Line 128: bricks = []
All the previous comments apply to this method too.
Line 152: bricks = []
All the previous comments apply to this method too.
--
To view, visit http://gerrit.ovirt.org/6374
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8769c2da299a598ed7500c4780a3329972f4a8ac
Gerrit-PatchSet: 2
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: Vijay Bellur <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
