Code review comment for lp:~dpigott/lava-scheduler-tool/fix821400-add-token-file

Revision history for this message
Dave Pigott (dpigott) wrote :

> We can probably remove that line
>
> 81 +# server = AuthenticatingServerProxy(
> 82 +# self.args.SERVER, auth_backend=KeyringAuthBackend())

I thought I had before I checked in. It certainly went just after! I'll push it up again.

> While this looks okay I think parsing the URL would be saner for bogus URLs.

In what way do you mean? This is just so that people can put in a truncated URL. Are you suggesting I should verify that the URL is actually a legitimate structure?

> 22 +def normalize_xmlrpc_url(uri):
> 23 + if '://' not in uri:
> 24 + uri = 'http://' + uri
> 25 + if not uri.endswith('/'):
> 26 + uri += '/'
> 27 + if not uri.endswith('/RPC2/'):
> 28 + uri += 'RPC2/'
> 29 + return uri

> Instead of checking this manually you can use MutuallyExclusiveGroup from
> argparse. It makes --help document this and allows you to express the same
> limitation without client-side code. See:
> http://argparse.googlecode.com/svn/trunk/doc/other-methods.html#mutual-
> exclusion
>
> 56 + if parsed_host.password:
> 57 + raise LavaCommandError(
> 58 + "Token specified in url but --token-file also passed.");

OK. Noted.

« Back to merge proposal