Alon Bar-Lev has posted comments on this change. Change subject: vdsClient: file based mechanism to provide password ......................................................................
Patch Set 10: (4 comments) http://gerrit.ovirt.org/#/c/24733/10/client/vdsClient.py File client/vdsClient.py: Line 102: print Line 103: print "Password can be provided as command line argument, path to" Line 104: print "file with password or environment variable by providing" Line 105: print "auth=file:path or auth=env:name or auth=pass:password" Line 106: > currently the vdsClient shows all the commands' info in its usage print, so no... you should print a hint in each command, and the entire description at one place. Line 107: Line 108: def printConf(conf): Line 109: try: Line 110: print "\n" + conf['vmId'] Line 162: return ret, length Line 163: Line 164: Line 165: def parseArgs(args): Line 166: con, length = parseOptions(args[2]) > what does 'con' stand for? sorry, it is a mistake, it should be args Line 167: if 'auth' in con: Line 168: con['password'] = getPassword(con['auth']) Line 169: return con Line 170: Line 168: con['password'] = getPassword(con['auth']) Line 169: return con Line 170: Line 171: Line 172: def getPassword(string, prefix=''): > the function can get only the value without the prefix, you already splitte I left it as it may be handy, it is not actually used. prefix is not split it is just remove, see below. Line 173: ret = None Line 174: (method, value) = string[len(prefix):].split(':', 1) Line 175: if method == 'file': Line 176: with open(value) as f: Line 2015: 'Sends reset signal to the vm' Line 2016: )), Line 2017: 'setVmTicket': (serv.do_setVmTicket, Line 2018: ('<vmId> <password> <sec> [disconnect|keep|fail], ' Line 2019: '[params={}] [args]', > you need to explain args somehow correct, see above note in usage, it should be something like "extra args, see extra args section" Line 2020: 'Set the password to the vm display for the next ' Line 2021: '<sec> seconds.', Line 2022: 'Optional argument instructs spice regarding ' Line 2023: 'currently-connected client.', -- To view, visit http://gerrit.ovirt.org/24733 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I363a16e6a7872ca05e19d5f520bdba90fb492374 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Yedidyah Bar David <[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
