launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/maas/script_atomic_write into lp:maas.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~julian-edwards/maas/script_atomic_write/+merge/122230

This adds a script wrapper around atomic_write so that we can call it using sudo.  Part one of the fix for writing the dhcpd config.
-- 
https://code.launchpad.net/~julian-edwards/maas/script_atomic_write/+merge/122230
Your team MAAS Maintainers is requested to review the proposed merge of lp:~julian-edwards/maas/script_atomic_write into lp:maas.

=== modified file 'src/provisioningserver/__main__.py'
--- src/provisioningserver/__main__.py	2012-08-19 23:57:42 +0000
+++ src/provisioningserver/__main__.py	2012-08-31 10:25:27 +0000
@@ -15,7 +15,7 @@
 import provisioningserver.dhcp.writer
 import provisioningserver.pxe.install_bootloader
 import provisioningserver.pxe.install_image
-from provisioningserver.utils import MainScript
+from provisioningserver.utils import AtomicWriteScript, MainScript
 
 
 main = MainScript(__doc__)
@@ -28,4 +28,7 @@
 main.register(
     "generate-dhcp-config",
     provisioningserver.dhcp.writer)
+main.register(
+    "atomic_write",
+    AtomicWriteScript)
 main()

=== modified file 'src/provisioningserver/tests/test_utils.py'
--- src/provisioningserver/tests/test_utils.py	2012-08-31 07:23:19 +0000
+++ src/provisioningserver/tests/test_utils.py	2012-08-31 10:25:27 +0000
@@ -20,7 +20,11 @@
 from random import randint
 import stat
 import StringIO
-from subprocess import CalledProcessError
+from subprocess import (
+    CalledProcessError,
+    PIPE,
+    Popen,
+    )
 import sys
 import time
 import types
@@ -28,9 +32,15 @@
 from maastesting.factory import factory
 from maastesting.fakemethod import FakeMethod
 from maastesting.testcase import TestCase
-from mock import Mock
+<<<<<<< TREE
+from mock import Mock
+=======
+from mock import Mock
+import provisioningserver
+>>>>>>> MERGE-SOURCE
 from provisioningserver.utils import (
     ActionScript,
+    AtomicWriteScript,
     atomic_write,
     get_mtime,
     incremental_write,
@@ -40,8 +50,15 @@
     Safe,
     ShellTemplate,
     )
+<<<<<<< TREE
 from testtools.matchers import FileContains
 from testtools.testcase import ExpectedException
+=======
+from testtools.matchers import (
+    FileContains,
+    MatchesStructure,
+    )
+>>>>>>> MERGE-SOURCE
 
 
 class TestSafe(TestCase):
@@ -390,3 +407,66 @@
         self.assertEqual(
             {"config_file": dummy_config_file, "handler": handler},
             vars(namespace))
+
+
+class TestAtomicWriteScript(TestCase):
+
+    def setUp(self):
+        super(TestAtomicWriteScript, self).setUp()
+        # Silence ArgumentParser.
+        self.patch(sys, "stdout", StringIO.StringIO())
+        self.patch(sys, "stderr", StringIO.StringIO())
+
+    def get_parser(self):
+        parser = ArgumentParser()
+        AtomicWriteScript.add_arguments(parser)
+        return parser
+
+    def test_arg_setup(self):
+        parser = self.get_parser()
+        filename = factory.getRandomString()
+        args = parser.parse_args((
+            '--no-overwrite',
+            '--filename', filename))
+        self.assertThat(
+            args, MatchesStructure.byEquality(
+                no_overwrite=True,
+                filename=filename))
+
+    def test_filename_arg_required(self):
+        parser = self.get_parser()
+        self.assertRaises(SystemExit, parser.parse_args, ('--no-overwrite',))
+
+    def test_no_overwrite_defaults_to_false(self):
+        parser = self.get_parser()
+        filename = factory.getRandomString()
+        args = parser.parse_args(('--filename', filename))
+        self.assertFalse(args.no_overwrite)
+
+    def test_script_executable(self):
+        dev_root = os.path.join(
+            os.path.dirname(provisioningserver.__file__),
+            os.pardir, os.pardir)
+        content = factory.getRandomString()
+        test_data_file = self.make_file(contents=content)
+        script = ["%s/bin/maas-provision" % dev_root, 'atomic_write']
+        target_file = self.make_file()
+        script.extend(('--filename', target_file))
+        with open(test_data_file, "rb") as stdin:
+            cmd = Popen(
+                script, stdin=stdin, stdout=PIPE,
+                env=dict(PYTHONPATH=":".join(sys.path)))
+            cmd.communicate()
+        self.assertThat(target_file, FileContains(content))
+
+    def test_passes_overwrite_flag(self):
+        content = factory.getRandomString()
+        self.patch(sys, "stdin", StringIO.StringIO(content))
+        parser = self.get_parser()
+        filename = factory.getRandomString()
+        args = parser.parse_args(('--filename', filename, '--no-overwrite'))
+        mock = Mock()
+        self.patch(provisioningserver.utils, 'atomic_write', mock)
+        AtomicWriteScript.run(args)
+        
+        mock.assert_called_once_with(content, filename, overwrite=False)

=== modified file 'src/provisioningserver/utils.py'
--- src/provisioningserver/utils.py	2012-08-31 06:59:14 +0000
+++ src/provisioningserver/utils.py	2012-08-31 10:25:27 +0000
@@ -302,3 +302,33 @@
             "-c", "--config-file", metavar="FILENAME",
             help="Configuration file to load [%(default)s].",
             default=Config.DEFAULT_FILENAME)
+
+
+class AtomicWriteScript:
+    """Wrap the atomic_write function turning it into an ActionScript.
+    
+    To use:
+    >>> main = MainScript(atomic_write.__doc__)
+    >>> main.register("myscriptname", AtomicWriteScript)
+    >>> main()
+    """
+
+    @staticmethod
+    def add_arguments(parser):
+        """Initialise options for writing files atomically.
+
+        :param parser: An instance of :class:`ArgumentParser`.
+        """
+        parser.add_argument(
+            "--no-overwrite", action="store_true", required=False,
+            default=False, help="Don't overwrite file if it exists")
+        parser.add_argument(
+            "--filename", action="store", required=True, help=(
+            "The name of the file in which to store contents of stdin"))
+
+    @staticmethod
+    def run(args):
+        """Take content from stdin and write it atomically to a file."""
+        content = sys.stdin.read()
+        atomic_write(
+            content, args.filename, overwrite=not args.no_overwrite)