Alon Bar-Lev has posted comments on this change. Change subject: vdsClient: file based mechanism to provide password ......................................................................
Patch Set 6: (3 comments) http://gerrit.ovirt.org/#/c/24733/6/client/vdsClient.py File client/vdsClient.py: Line 367: print response['status']['message'] Line 368: sys.exit(response['status']['code']) Line 369: Line 370: def do_setVmTicket(self, args): Line 371: if args[-1].startswith('auth'): not sure why -1 is hardcoded, don't you need to iterate throught all args? Line 372: otp64 = getPassword(args[-1][5:]) Line 373: if len(args) == 3: Line 374: vmId, secs = args[:2] Line 375: connAct = 'disconnect' Line 368: sys.exit(response['status']['code']) Line 369: Line 370: def do_setVmTicket(self, args): Line 371: if args[-1].startswith('auth'): Line 372: otp64 = getPassword(args[-1][5:]) please hold auth in constant and use len(auth) to parse, better is to pass the prefix into getPassword as I suggested so you can remove it at one place nicely. Line 373: if len(args) == 3: Line 374: vmId, secs = args[:2] Line 375: connAct = 'disconnect' Line 376: params = {} Line 475: Line 476: def desktopLogin(self, args): Line 477: vmId, domain, user, password = tuple(args) Line 478: if args[-1].startswith('auth'): Line 479: password = getPassword(password[5:]) please do not hard code 5, why -1? don't you want bbb=zzz,auth=xxx,aaa=yyy in future? Line 480: response = self.s.desktopLogin(vmId, domain, user, password) Line 481: print response['status']['message'] Line 482: sys.exit(response['status']['code']) Line 483: -- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[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
