launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/remove-pxe-title into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~allenap/maas/remove-pxe-title/+merge/117940
-- 
https://code.launchpad.net/~allenap/maas/remove-pxe-title/+merge/117940
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/remove-pxe-title into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-08-01 15:37:57 +0000
+++ src/maasserver/api.py	2012-08-02 16:23:38 +0000
@@ -133,7 +133,6 @@
 from piston.models import Token
 from piston.resource import Resource
 from piston.utils import rc
-from provisioningserver.pxe.config import render_pxe_config
 
 
 dispatch_methods = {
@@ -1107,14 +1106,12 @@
         will be the "default" one which boots into an enlistment image.
     :param arch: Main machine architecture.
     :param subarch: Sub-architecture, or "generic" if there is none.
-    :param title: Title that the node should show in its PXE menu.
     :param append: Additional parameters to append to the kernel command
         line.
     """
     mac = request.GET.get('mac', None)
     arch = get_mandatory_param(request.GET, 'arch')
     subarch = request.GET.get('subarch', 'generic')
-    title = get_mandatory_param(request.GET, 'title')
     append = get_mandatory_param(request.GET, 'append')
 
     # See if we have a record of this MAC address, and thus node.
@@ -1133,7 +1130,7 @@
     release = "precise"
 
     params = dict(
-        title=title, arch=arch, subarch=subarch, release=release,
+        arch=arch, subarch=subarch, release=release,
         purpose=get_boot_purpose(node), append=append)
 
     return HttpResponse(

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-08-02 12:19:31 +0000
+++ src/maasserver/tests/test_api.py	2012-08-02 16:23:38 +0000
@@ -87,7 +87,6 @@
     POWER_TYPE_CHOICES,
     )
 from testtools.matchers import (
-    Contains,
     Equals,
     MatchesListwise,
     StartsWith,
@@ -2261,7 +2260,6 @@
                 'arch': "armhf",
                 'subarch': "armadaxp",
                 'mac': factory.make_mac_address().mac_address,
-                'title': factory.make_name("Menu"),
                 'append': factory.make_name("append"),
             }
 
@@ -2286,7 +2284,7 @@
             response)
         params_out = json.loads(response.content)
         # Some parameters are unchanged.
-        params_unchanged = {"arch", "subarch", "title"}
+        params_unchanged = {"arch", "subarch"}
         self.assertEqual(
             {name: params_in[name] for name in params_unchanged},
             {name: params_out[name] for name in params_unchanged})
@@ -2313,7 +2311,6 @@
             'arch': httplib.BAD_REQUEST,
             'subarch': httplib.OK,
             'mac': httplib.OK,
-            'title': httplib.BAD_REQUEST,
             'append': httplib.BAD_REQUEST,
             }
         observed = {

=== modified file 'src/provisioningserver/pxe/config.py'
--- src/provisioningserver/pxe/config.py	2012-08-01 16:19:50 +0000
+++ src/provisioningserver/pxe/config.py	2012-08-02 16:23:38 +0000
@@ -34,10 +34,9 @@
 
 
 def render_pxe_config(
-    title, arch, subarch, release, purpose, append, bootpath, **extra):
+    arch, subarch, release, purpose, append, bootpath, **extra):
     """Render a PXE configuration file as a unicode string.
 
-    :param title: Title that the node should show on its boot menu.
     :param arch: Main machine architecture.
     :param subarch: Sub-architecture, or "generic" if there is none.
     :param release: The OS release, e.g. "precise".
@@ -54,6 +53,5 @@
         "initrd": "%s/initrd.gz" % image_dir,
         "kernel": "%s/kernel" % image_dir,
         "relpath": partial(posixpath.relpath, start=bootpath),
-        "title": title,
         }
     return template.substitute(namespace)

=== modified file 'src/provisioningserver/pxe/tests/test_config.py'
--- src/provisioningserver/pxe/tests/test_config.py	2012-08-02 12:40:52 +0000
+++ src/provisioningserver/pxe/tests/test_config.py	2012-08-02 16:23:38 +0000
@@ -34,7 +34,6 @@
         # Given the right configuration options, the PXE configuration is
         # correctly rendered.
         options = {
-            "title": factory.make_name("title"),
             "arch": factory.make_name("arch"),
             "subarch": factory.make_name("subarch"),
             "release": factory.make_name("release"),
@@ -68,7 +67,6 @@
     def test_render_with_extra_arguments_does_not_affect_output(self):
         # render_pxe_config() allows any keyword arguments as a safety valve.
         options = {
-            "title": factory.make_name("title"),
             "arch": factory.make_name("arch"),
             "subarch": factory.make_name("subarch"),
             "release": factory.make_name("release"),

=== modified file 'src/provisioningserver/tests/test_tftp.py'
--- src/provisioningserver/tests/test_tftp.py	2012-08-02 09:53:27 +0000
+++ src/provisioningserver/tests/test_tftp.py	2012-08-02 16:23:38 +0000
@@ -134,10 +134,8 @@
         arch = factory.make_name("arch").encode("ascii")
         subarch = factory.make_name("subarch").encode("ascii")
         mac = factory.getRandomMACAddress(b"-")
-        title = factory.make_name("menu-title").encode("ascii")
         append = factory.make_name("append").encode("ascii")
-        backend_url = b"http://example.com/?"; + (
-            urlencode({b"title": title, b"append": append}))
+        backend_url = b"http://example.com/?"; + urlencode({b"append": append})
         backend = TFTPBackend(self.make_dir(), backend_url)
         # params is an example of the parameters obtained from a request.
         params = {"arch": arch, "subarch": subarch, "mac": mac}
@@ -148,7 +146,6 @@
             ("append", append),
             ("arch", arch),
             ("subarch", subarch),
-            ("title", title),
             ("mac", mac),
             ]
         self.assertItemsEqual(query_expected, query)
@@ -210,12 +207,11 @@
         fake_api_params.update(
             append=factory.make_name("append"),
             purpose=factory.make_name("purpose"),
-            release=factory.make_name("release"),
-            title=factory.make_name("title"))
-        # Add a title to the first set of parameters. This will later help
+            release=factory.make_name("release"))
+        # Add a purpose to the first set of parameters. This will later help
         # demonstrate that the API parameters take precedence over the file
         # path parameters.
-        fake_params["title"] = factory.make_name("original-title")
+        fake_params["purpose"] = factory.make_name("original-purpose")
         # Stub get_page to return the fake API configuration parameters.
         fake_api_params_json = json.dumps(fake_api_params)
         backend.get_page = lambda url: succeed(fake_api_params_json)

=== modified file 'src/provisioningserver/tftp.py'
--- src/provisioningserver/tftp.py	2012-08-01 14:26:55 +0000
+++ src/provisioningserver/tftp.py	2012-08-02 16:23:38 +0000
@@ -114,11 +114,8 @@
         :param params: A dict, or iterable suitable for updating a dict, of
             additional query parameters.
         """
-        # TODO: update query defaults.
-        query = {
-            b"title": b"",
-            b"append": b"",
-            }
+        # Query defaults.
+        query = {b"append": b""}
         # Merge parameters from the generator URL.
         query.update(parse_qsl(self.generator_url.query))
         # Merge parameters obtained from the request.