Bala.FA has posted comments on this change.

Change subject: gluster: Adds new verbs to setup gluster geo-replication.
......................................................................


Patch Set 12: Code-Review-1

(5 comments)

Sorry.  I missed out earlier in the review.

https://gerrit.ovirt.org/#/c/38228/12/vdsm/gluster/api.py
File vdsm/gluster/api.py:

Line 82: remoteUserName
remoteUserName has to be userName as this key update is done local cluster only.


Line 92: authKeys
rename this to authKeysFile


Line 108: existingKeyLines
can you do this in try..except block by checking EEXIST than os.path.exists()?  
The try block needs to be out of current try block


Line 117: join
As outLines is a list of string, does ''.join(...) work?


Line 569: remoteUserName
its not remoteUserName instead its userName.

Please change this everywhere


-- 
To view, visit https://gerrit.ovirt.org/38228
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I06a64b8f0db8846ca2697b9f0ec69cab2d3d6b1e
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N <[email protected]>
Gerrit-Reviewer: Aravinda VK <[email protected]>
Gerrit-Reviewer: Bala.FA <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Darshan N <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Timothy Asir <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to