Nir Soffer has posted comments on this change. Change subject: v2v: Add convertExternalVm for XmlRpc ......................................................................
Patch Set 4: (4 comments) https://gerrit.ovirt.org/#/c/40003/4/client/vdsClient.py File client/vdsClient.py: Line 173: return getPassword(args['auth']) Line 174: return default Line 175: Line 176: Line 177: def parsePassword(string): Having both parsePassword and getPassword is little bit confusing. Line 178: if string.startswith('auth='): Line 179: return getPassword(string.split('=')[1]) Line 180: return string Line 181: Line 1921: def convertExternalVm(self, args): Line 1922: validateArgTypes(args, [str, str, str, parse_dict, str], Line 1923: requiredArgsNumber=5) Line 1924: uri, username, auth, vminfo, jobid = args Line 1925: passwd = parsePassword(auth) Instead of adding new confusing parsePassword function, you can use the existing infrastructure: d = parseArgs(auth) password = getAuthFromArgs(d, auth) It is very good infrastructure, but we should fix all places parsing auth=, not add multiple incompatible ways to do the same thing. Line 1926: response = self.s.convertExternalVm(uri, username, passwd, vminfo, Line 1927: jobid) Line 1928: if response['status']['code'] != 0: Line 1929: return response['status']['code'], response['status']['message'] Line 2804: ' uri: uri of external system (vmware etc)', Line 2805: ' username: login name for given uri', Line 2806: ' auth: password for given uri, can be: ', Line 2807: ' plain text password or auth=file:path ' Line 2808: 'or auth=env:name or auth=pass:password', Should be indented so it aligns with "plain text..." Line 2809: ' vminfo: Python dictionary, parameter for import:', Line 2810: ' {"vmName": "name",', Line 2811: ' "poolId":, "<UUID>",', Line 2812: ' "domainId": "<UUID",', Line 2805: ' username: login name for given uri', Line 2806: ' auth: password for given uri, can be: ', Line 2807: ' plain text password or auth=file:path ' Line 2808: 'or auth=env:name or auth=pass:password', Line 2809: ' vminfo: Python dictionary, parameter for import:', This must be quoted, so it will parse as single command line argument. Line 2810: ' {"vmName": "name",', Line 2811: ' "poolId":, "<UUID>",', Line 2812: ' "domainId": "<UUID",', Line 2813: ' "disks": [{"volumeId": "<UUID>",', -- To view, visit https://gerrit.ovirt.org/40003 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I94d7886e1295f5e98d0fafc8837567ff383a6c7a Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches