launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/bug-1030882 into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1030882 in MAAS: "URL in PXE config "auto url=..." fragment does not include scheme or domain"
  https://bugs.launchpad.net/maas/+bug/1030882

For more details, see:
https://code.launchpad.net/~rvb/maas/bug-1030882/+merge/117286

This came up during a call with Gavin. This solves one of the bugs he filed after testing things in the lab.  More precisely it fixes the API methods compose_enlistment_preseed_url and compose_preseed_url so that they now return absolute urls.
-- 
https://code.launchpad.net/~rvb/maas/bug-1030882/+merge/117286
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/bug-1030882 into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-07-26 07:54:27 +0000
+++ src/maasserver/api.py	2012-07-30 16:12:18 +0000
@@ -85,7 +85,6 @@
     PermissionDenied,
     ValidationError,
     )
-from django.core.urlresolvers import reverse
 from django.forms.models import model_to_dict
 from django.http import (
     HttpResponse,
@@ -124,6 +123,7 @@
     Node,
     NodeGroup,
     )
+from maasserver.utils import absolute_reverse
 from piston.doc import generate_doc
 from piston.handler import (
     AnonymousBaseHandler,
@@ -1050,16 +1050,18 @@
     """Compose enlistment preseed URL."""
     # Always uses the latest version of the metadata API.
     version = 'latest'
-    return "%s?op=get_enlist_preseed" % reverse(
-        'metadata-enlist-preseed', args=[version])
+    return absolute_reverse(
+        'metadata-enlist-preseed', args=[version],
+        query={'op': 'get_enlist_preseed'})
 
 
 def compose_preseed_url(node):
     """Compose a metadata URL for `node`'s preseed data."""
     # Always uses the latest version of the metadata API.
     version = 'latest'
-    return "%s?op=get_preseed" % reverse(
-        'metadata-node-by-id', args=[version, node.system_id])
+    return absolute_reverse(
+        'metadata-node-by-id', args=[version, node.system_id],
+        query={'op': 'get_preseed'})
 
 
 def compose_preseed_kernel_opt(mac):

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-07-26 07:54:27 +0000
+++ src/maasserver/tests/test_api.py	2012-07-30 16:12:18 +0000
@@ -2301,6 +2301,12 @@
             (httplib.OK, get_enlist_preseed()),
             (response.status_code, response.content))
 
+    def test_compose_enlistment_preseed_url_returns_absolute_link(self):
+        url = 'http://%s' % factory.make_name('host')
+        self.patch(settings, 'DEFAULT_MAAS_URL', url)
+        self.assertThat(
+                api.compose_enlistment_preseed_url(), StartsWith(url))
+
     def test_compose_preseed_url_links_to_preseed_for_node(self):
         node = factory.make_node()
         response = self.client.get(api.compose_preseed_url(node))
@@ -2308,6 +2314,13 @@
             (httplib.OK, get_preseed(node)),
             (response.status_code, response.content))
 
+    def test_compose_preseed_url_returns_absolute_link(self):
+        url = 'http://%s' % factory.make_name('host')
+        self.patch(settings, 'DEFAULT_MAAS_URL', url)
+        node = factory.make_node()
+        self.assertThat(
+                api.compose_preseed_url(node), StartsWith(url))
+
     def test_compose_preseed_kernel_opt_returns_option_for_known_node(self):
         mac = factory.make_mac_address()
         self.assertEqual(