Merge lp:~dpigott/lava-scheduler-tool/fix821400-add-token-file into lp:lava-scheduler-tool

[Dave Pigott]

Added code to support --token-file to the submit-job command. In the process realised that cancel-job requires the same functionality and that there is now code duplication between auth-add and submit-job. Will open that work as a separate bug.

[Zygmunt Krynicki (zyga)]

We can probably remove that line

81 +# server = AuthenticatingServerProxy(
82 +# self.args.SERVER, auth_backend=KeyringAuthBackend())

While this looks okay I think parsing the URL would be saner for bogus URLs.
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.");

[Dave Pigott (dpigott)]

> 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.

[Zygmunt Krynicki (zyga)]

> > 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?

Well instead of using "://" in url and things like that we could parse the url with things like urlparse/urlsplit and then only act on the path component. I'm not advocating for it but it would seem cleaner.

[Dave Pigott (dpigott)]

> 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.");

Actually, when I came to look at this, it doesn't apply. One is a command line option, the other is a part of the url as broken up by url parse, so I can't make a mutual exclusion. Pity. It's a really nice feature.

[Zygmunt Krynicki (zyga)]

> > 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.");
>
> Actually, when I came to look at this, it doesn't apply. One is a command line
> option, the other is a part of the url as broken up by url parse, so I can't
> make a mutual exclusion. Pity. It's a really nice feature.

Ah, quite right. My mistake, sorry about that.

ZK

[Michael Hudson-Doyle (mwhudson)]

Apologies for not commenting earlier. I really thought I had :/

This change means that the keyring backend will never be used! I don't think that's right. I've made some changes in https://code.launchpad.net/~dpigott/lava-scheduler-tool/fix821400-add-token-file (extracting fiddling around for a token to a function, and use the keyring backend if no token is otherwise supplied). Perhaps a function that gets an AuthenticatingServerProxy given a server url and a token file name (or even an argparse options object) needs to be added to lava-tool...

review: Needs Fixing

[Dave Pigott (dpigott)]

> Apologies for not commenting earlier. I really thought I had :/
>
> This change means that the keyring backend will never be used! I don't think
> that's right. I've made some changes in https://code.launchpad.net/~dpigott
> /lava-scheduler-tool/fix821400-add-token-file (extracting fiddling around for
> a token to a function, and use the keyring backend if no token is otherwise
> supplied).

I think I've misunderstood something. Pulled that branch and it looks like what I checked in. Is that not what you meant?

> Perhaps a function that gets an AuthenticatingServerProxy given a
> server url and a token file name (or even an argparse options object) needs to
> be added to lava-tool...

That's kind of the direction I've been going in on the cancel-job addition and code consolidation.

[Michael Hudson-Doyle (mwhudson)]

On Mon, 19 Sep 2011 08:02:34 -0000, Dave Pigott <email address hidden> wrote:
> > Apologies for not commenting earlier. I really thought I had :/
> >
> > This change means that the keyring backend will never be used! I don't think
> > that's right. I've made some changes in https://code.launchpad.net/~dpigott
> > /lava-scheduler-tool/fix821400-add-token-file (extracting fiddling around for
> > a token to a function, and use the keyring backend if no token is otherwise
> > supplied).
>
> I think I've misunderstood something. Pulled that branch and it looks like what I checked in. Is that not what you meant?

Aaargh I meant
https://code.launchpad.net/~mwhudson/lava-scheduler-tool/fix821400-add-token-file

Sorry about that.

> > Perhaps a function that gets an AuthenticatingServerProxy given a
> > server url and a token file name (or even an argparse options object) needs to
> > be added to lava-tool...
>
> That's kind of the direction I've been going in on the cancel-job addition and code consolidation.

Cool.

Cheers,
mwh

[Zygmunt Krynicki (zyga)]

Michael, what is the status of this MP, is it going to land or can it be rejected?

review: Needs Information

[Michael Hudson-Doyle (mwhudson)]

There have been no revisions pushed since I made my comments, I've been waiting for Dave to take a look at my branch.

[Dave Pigott (dpigott)]

Other things got in the way. When everything quietens down here I'll review. I think Michael's branch is probably right, but need to look.

Unmerged revisions

9. By Dave Pigott on 2011-09-06

Added authentiction token file support to submit-job

=== modified file 'lava_scheduler_tool/commands.py'
--- lava_scheduler_tool/commands.py	2011-08-19 04:32:11 +0000
+++ lava_scheduler_tool/commands.py	2011-09-06 10:45:37 +0000
@@ -16,11 +16,27 @@
 # You should have received a copy of the GNU Lesser General Public License
 # along with lava-scheduler-tool.  If not, see <http://www.gnu.org/licenses/>.
 
+import getpass
+import urlparse
+import xmlrpclib
 
-from lava_tool.authtoken import AuthenticatingServerProxy, KeyringAuthBackend
-from lava_tool.interface import Command
+from lava_tool.authtoken import (
+    AuthenticatingServerProxy,
+    KeyringAuthBackend,
+    MemoryAuthBackend,
+    )
+from lava_tool.interface import Command, LavaCommandError
 from lava_tool.commands import ExperimentalCommandMixIn
 
+def normalize_xmlrpc_url(uri):
+    if '://' not in uri:
+        uri = 'http://' + uri
+    if not uri.endswith('/'):
+        uri += '/'
+    if not uri.endswith('/RPC2/'):
+        uri += 'RPC2/'
+    return uri
+
 
 class submit_job(ExperimentalCommandMixIn, Command):
     """
@@ -32,11 +48,51 @@
         super(submit_job, cls).register_arguments(parser)
         parser.add_argument("SERVER")
         parser.add_argument("JSON_FILE")
+        parser.add_argument(
+            "--token-file", default=None,
+            help="Read the password from a file rather than prompting for it.")
 
     def invoke(self):
+        uri = normalize_xmlrpc_url(self.args.SERVER)
+        parsed_host = urlparse.urlparse(uri)
+
+        if parsed_host.username:
+            username = parsed_host.username
+        else:
+            username = getpass.getuser()
+
+        host = parsed_host.hostname
+        if parsed_host.port:
+            host += ':' + str(parsed_host.port)
+
+        if self.args.token_file:
+            if parsed_host.password:
+                raise LavaCommandError(
+                    "Token specified in url but --token-file also passed.");
+            else:
+                try:
+                    token_file = open(self.args.token_file)
+                except IOError as ex:
+                    raise LavaCommandError(
+                        "opening %r failed: %s" % (self.args.token_file, ex))
+                token = token_file.read().strip()
+        else:
+            if parsed_host.password:
+                token = parsed_host.password
+            else:
+                token = getpass.getpass("Paste token for %s: " % uri)
+        
+        userless_uri = '%s://%s%s' % (
+            parsed_host.scheme, host, parsed_host.path)
+
         self.print_experimental_notice()
+
         server = AuthenticatingServerProxy(
-            self.args.SERVER, auth_backend=KeyringAuthBackend())
+            uri, auth_backend = MemoryAuthBackend(
+                [(username, userless_uri, token)]))
+#        server = AuthenticatingServerProxy(
+#            self.args.SERVER, auth_backend=KeyringAuthBackend())
+
         with open(self.args.JSON_FILE, 'rb') as stream:
             command_text = stream.read()
         print "submitted as job id:", server.scheduler.submit_job(command_text)