launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/infer-api-uri into lp:maas.

Commit message:
Infer the API version, and thus URL, at the command-line.

Previously it was necessary to specify the full URL to the API root, e.g. http://example.com/api/1.0.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1061731 in MAAS: "MAAS does not infer the API url when one connects to the MAAS url using the cli."
  https://bugs.launchpad.net/maas/+bug/1061731

For more details, see:
https://code.launchpad.net/~allenap/maas/infer-api-uri/+merge/128496
-- 
https://code.launchpad.net/~allenap/maas/infer-api-uri/+merge/128496
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/infer-api-uri into lp:maas.

=== modified file 'src/maascli/cli.py'
--- src/maascli/cli.py	2012-10-08 10:27:52 +0000
+++ src/maascli/cli.py	2012-10-08 13:45:31 +0000
@@ -20,7 +20,7 @@
 from maascli.command import Command
 from maascli.config import ProfileConfig
 from maascli.utils import (
-    ensure_trailing_slash,
+    api_url,
     parse_docstring,
     safe_name,
     )
@@ -41,9 +41,10 @@
                 "server and credentials within this tool."
                 ))
         parser.add_argument(
-            "url", help=(
-                "The URL of the remote API, e.g. "
-                "http://example.com/MAAS/api/1.0/";))
+            "url", type=api_url, help=(
+                "The URL of the remote API, e.g. http://example.com/MAAS/ "
+                "or http://example.com/MAAS/api/1.0/ if you wish to specify "
+                "the API version."))
         parser.add_argument(
             "credentials", nargs="?", default=None, help=(
                 "The credentials, also known as the API key, for the "
@@ -61,11 +62,8 @@
         # Try and obtain credentials interactively if they're not given, or
         # read them from stdin if they're specified as "-".
         credentials = obtain_credentials(options.credentials)
-        # Normalise the remote service's URL.
-        url = ensure_trailing_slash(options.url)
         # Get description of remote API.
-        insecure = options.insecure
-        description = fetch_api_description(url, insecure)
+        description = fetch_api_description(options.url, options.insecure)
         # Save the config.
         profile_name = options.profile_name
         with ProfileConfig.open() as config:
@@ -73,7 +71,7 @@
                 "credentials": credentials,
                 "description": description,
                 "name": profile_name,
-                "url": url,
+                "url": options.url,
                 }
 
 

=== modified file 'src/maascli/tests/test_utils.py'
--- src/maascli/tests/test_utils.py	2012-10-07 22:06:22 +0000
+++ src/maascli/tests/test_utils.py	2012-10-08 13:45:31 +0000
@@ -18,9 +18,11 @@
 from maascli import utils
 from maastesting.testcase import TestCase
 from testtools.matchers import (
+    AfterPreprocessing,
     Equals,
     IsInstance,
     MatchesAll,
+    MatchesListwise,
     )
 
 
@@ -194,3 +196,23 @@
         name, value = unquote(name), unquote(value)
         name, value = smart_unicode(name), smart_unicode(value)
         self.assertEqual(data, [(name, value)])
+
+    def test_api_url(self):
+        transformations = {
+            "http://example.com/":
+                "http://example.com/api/1.0/";,
+            "http://example.com/foo":
+                "http://example.com/foo/api/1.0/";,
+            "http://example.com/foo/":
+                "http://example.com/foo/api/1.0/";,
+            "http://example.com/api/7.9":
+                "http://example.com/api/7.9/";,
+            "http://example.com/api/7.9/":
+                "http://example.com/api/7.9/";,
+            }.items()
+        urls = [url for url, url_out in transformations]
+        expected = [
+            AfterPreprocessing(utils.api_url, Equals(url_out))
+            for url, url_out in transformations
+            ]
+        self.assertThat(urls, MatchesListwise(expected))

=== modified file 'src/maascli/utils.py'
--- src/maascli/utils.py	2012-10-07 22:06:22 +0000
+++ src/maascli/utils.py	2012-10-08 13:45:31 +0000
@@ -25,6 +25,7 @@
     )
 import re
 from urllib import quote_plus
+from urlparse import urlparse
 
 
 re_paragraph_splitter = re.compile(
@@ -85,6 +86,22 @@
     return (string + slash) if not string.endswith(slash) else string
 
 
+def api_url(string):
+    """Ensure that `string` looks like a URL to the API.
+
+    This ensures that the API version is specified explicitly (i.e. the path
+    ends with /api/{version}). If not, version 1.0 is selected. It also
+    ensures that the path ends with a forward-slash.
+
+    This is suitable for use as an argument type with argparse.
+    """
+    url = urlparse(string)
+    url = url._replace(path=ensure_trailing_slash(url.path))
+    if re.search("/api/[0-9.]+/?$", url.path) is None:
+        url = url._replace(path=url.path + "api/1.0/")
+    return url.geturl()
+
+
 def urlencode(data):
     """A version of `urllib.urlencode` that isn't insane.