launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/fix-makefile-write-dhcp-stuff into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~allenap/maas/fix-makefile-write-dhcp-stuff/+merge/109324

This changes a few bits about the new DHCPConfigWriter.

- Renames write_dhcp_config.py to writer.py. It's in the
  provisioningserver.dhcp module, and it hardly needs explaining that
  it's about configuration.

- Removes the write_dhcp_config script entirely. It's not necessary -
  use bin/py -m provisioningserver/dhcp/writer instead - and is
  misleading because packages cannot use it anyway as it is generated
  by buildout.

- Remove all references to write_dhcp_config from Makefile.

- Remove the test for the write_dhcp_config script.

- Use stdout.write() to emit the config instead of print(); the latter
  adds an extra newline.

- Encodes the config as ASCII for the paranoid.

- Drive-by: restricts the scripts that the [database] part in
  buildout.cfg creates to just the bin/database script. Previously it
  was also generating a bin/postgresfixture script.

-- 
https://code.launchpad.net/~allenap/maas/fix-makefile-write-dhcp-stuff/+merge/109324
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/fix-makefile-write-dhcp-stuff into lp:maas.

=== modified file 'Makefile'
--- Makefile	2012-06-08 00:23:22 +0000
+++ Makefile	2012-06-08 10:31:19 +0000
@@ -21,7 +21,6 @@
     bin/twistd.pserv bin/test.pserv \
     bin/twistd.txlongpoll \
     bin/py bin/ipy \
-    bin/write_dhcp_config \
     $(js_enums)
 
 all: build doc
@@ -70,10 +69,6 @@
 	bin/buildout install repl
 	@touch --no-create bin/py bin/ipy
 
-bin/write_dhcp_config: bin/buildout buildout.cfg versions.cfg setup.py
-	bin/buildout install write-dhcp-config
-	@touch --no-create $@
-
 test: bin/test.maas bin/test.maastesting bin/test.pserv $(js_enums)
 	bin/test.maas
 	bin/test.maastesting
@@ -95,7 +90,7 @@
 lint-js:
 	@find $(sources) -type f -print0 | xargs -r0 $(pocketlint)
 
-check: clean build test
+check: clean test
 
 docs/api.rst: bin/maas src/maasserver/api.py syncdb
 	bin/maas generate_api_doc > $@

=== modified file 'buildout.cfg'
--- buildout.cfg	2012-06-07 05:54:29 +0000
+++ buildout.cfg	2012-06-08 10:31:19 +0000
@@ -9,7 +9,6 @@
   repl
   sphinx
   txlongpoll
-  write-dhcp-config
 extensions = buildout-versions
 buildout_versions_file = versions.cfg
 versions = versions
@@ -80,6 +79,7 @@
 extra-paths = ${common:extra-paths}
 interpreter =
 entry-points = database=postgresfixture.main:main
+scripts = database
 
 [maas]
 recipe = zc.recipe.egg
@@ -214,9 +214,3 @@
   txamqp
 entry-points = twistd.txlongpoll=twisted.scripts.twistd:run
 scripts = twistd.txlongpoll
-
-[write-dhcp-config]
-recipe = z3c.recipe.scripts
-extra-paths = ${common:extra-paths}
-entry-points = write_dhcp_config=provisioningserver.dhcp.write_dhcp_config:run
-scripts = write_dhcp_config

=== renamed file 'src/provisioningserver/dhcp/tests/test_write_dhcp_config.py' => 'src/provisioningserver/dhcp/tests/test_writer.py'
--- src/provisioningserver/dhcp/tests/test_write_dhcp_config.py	2012-06-07 07:53:25 +0000
+++ src/provisioningserver/dhcp/tests/test_writer.py	2012-06-08 10:31:19 +0000
@@ -1,7 +1,7 @@
 # Copyright 2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""Tests for write-dhcp-config.py"""
+"""Tests for `provisioningserver.dhcp.writer`."""
 
 from __future__ import (
     absolute_import,
@@ -13,19 +13,15 @@
 __all__ = []
 
 import os
-import subprocess
 
 from maastesting.matchers import ContainsAll
 from maastesting.testcase import TestCase
-from testtools.matchers import (
-    MatchesStructure,
-    )
-
-from provisioningserver.dhcp.write_dhcp_config import DHCPConfigWriter
-
-
-class TestModule(TestCase):
-    """Test the write-dhcp-config module."""
+from provisioningserver.dhcp.writer import DHCPConfigWriter
+from testtools.matchers import MatchesStructure
+
+
+class TestDHCPConfigWriter(TestCase):
+    """Test `DHCPConfigWriter`."""
 
     def test_arg_setup(self):
         writer = DHCPConfigWriter()
@@ -92,32 +88,3 @@
         writer.run(test_args)
 
         self.assertTrue(os.path.exists(outfile))
-
-
-class TestScriptExecutable(TestCase):
-    """Test that the actual script is executable."""
-
-    def test_script(self):
-        test_args = [
-            '--subnet', 'subnet',
-            '--subnet-mask', 'subnet-mask',
-            '--next-server', 'next-server',
-            '--broadcast-address', 'broadcast-address',
-            '--dns-servers', 'dns-servers',
-            '--gateway', 'gateway',
-            '--low-range', 'low-range',
-            '--high-range', 'high-range',
-            ]
-
-        exe = [os.path.join(
-            os.path.dirname(__file__),
-            os.pardir, os.pardir, os.pardir, os.pardir,
-            "bin", "write_dhcp_config")]
-
-        exe.extend(test_args)
-        output = subprocess.check_output(exe)
-
-        contains_all_params = ContainsAll(
-            ['subnet', 'subnet-mask', 'next-server', 'broadcast-address',
-            'dns-servers', 'gateway', 'low-range', 'high-range'])
-        self.assertThat(output, contains_all_params)

=== renamed file 'src/provisioningserver/dhcp/write_dhcp_config.py' => 'src/provisioningserver/dhcp/writer.py'
--- src/provisioningserver/dhcp/write_dhcp_config.py	2012-06-07 07:53:25 +0000
+++ src/provisioningserver/dhcp/writer.py	2012-06-08 10:31:19 +0000
@@ -12,8 +12,8 @@
 __metaclass__ = type
 __all__ = []
 
-
 import argparse
+from sys import stdout
 
 from provisioningserver.dhcp import config
 
@@ -73,20 +73,13 @@
     def run(self, argv=None):
         """Generate the config and write to stdout or a file as required."""
         args = self.parse_args(argv)
-        output = self.generate(args)
-        outfile = args.out_file
-        if outfile is not None:
-            with open(outfile, "w") as f:
+        output = self.generate(args).encode("ascii")
+        if args.out_file is not None:
+            with open(args.out_file, "wb") as f:
                 f.write(output)
         else:
-            print(output)
-
-
-def run():
-    """Entry point for scripts."""
-    writer = DHCPConfigWriter()
-    writer.run()
+            stdout.write(output)
 
 
 if __name__ == "__main__":
-    run()
+    DHCPConfigWriter().run()