launchpad-reviewers team mailing list archive

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

Raphaël Badin has proposed merging lp:~rvb/maas/yaml-safe-dump into lp:maas with lp:~rvb/maas/preseed-ui as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/yaml-safe-dump/+merge/111568

Use yaml.safe_{dump, load} instead of yaml.{dump, load}.  This is a tiny improvement done so that the preseed file will look 'right'.  yaml.dump uses python-specific tags to allow dumping python objects (same as pickle).  This is a functionality we don't need and which has a side effect: a prefix is added in front of unicode objects:

>>> import yaml
>>> yaml.dump({u'test' : u'testé'})
'{!!python/unicode \'test\': "test\\xE9"}\n'
>>> yaml.safe_dump({u'test' : u'testé'})
'{test: "test\\xE9"}\n'

= Pre-imp =

This was briefly discussed with Gavin when the two of us where working on getting the preseeds rendered in the UI and exposed over the metadata API.

= Notes =

Note that this was working fine because python's yaml module is used everywhere (i.e. to write and read these preseed files) but I think it's better to use only standard YAML tags to avoid confusion we a user will be looking at the preseed files.
-- 
https://code.launchpad.net/~rvb/maas/yaml-safe-dump/+merge/111568
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/yaml-safe-dump into lp:maas.

=== modified file 'etc/maas/commissioning-user-data'
--- etc/maas/commissioning-user-data	2012-04-17 06:33:43 +0000
+++ etc/maas/commissioning-user-data	2012-06-22 09:54:20 +0000
@@ -250,7 +250,7 @@
             url = url[7:]
         cfg_str = open(url,"r").read()
 
-    cfg = yaml.load(cfg_str)
+    cfg = yaml.safe_load(cfg_str)
 
     # support reading cloud-init config for MAAS datasource
     if 'datasource' in cfg:

=== modified file 'src/maasserver/provisioning.py'
--- src/maasserver/provisioning.py	2012-04-20 15:16:41 +0000
+++ src/maasserver/provisioning.py	2012-06-22 09:54:20 +0000
@@ -368,7 +368,7 @@
 
 def compose_commissioning_preseed(token):
     """Compose the preseed value for a Commissioning node."""
-    return "#cloud-config\n%s" % yaml.dump({
+    return "#cloud-config\n%s" % yaml.safe_dump({
         'datasource': {
             'MAAS': {
                 'metadata_url': get_metadata_server_url(),

=== modified file 'src/maasserver/tests/test_provisioning.py'
--- src/maasserver/tests/test_provisioning.py	2012-05-03 08:48:25 +0000
+++ src/maasserver/tests/test_provisioning.py	2012-06-22 09:54:20 +0000
@@ -93,7 +93,7 @@
 
     def test_compose_preseed_for_commissioning_node_produces_yaml(self):
         node = factory.make_node(status=NODE_STATUS.COMMISSIONING)
-        preseed = yaml.load(compose_preseed(node))
+        preseed = yaml.safe_load(compose_preseed(node))
         self.assertIn('datasource', preseed)
         self.assertIn('MAAS', preseed['datasource'])
         self.assertThat(
@@ -111,7 +111,7 @@
 
     def test_compose_preseed_for_commissioning_includes_metadata_url(self):
         node = factory.make_node(status=NODE_STATUS.COMMISSIONING)
-        preseed = yaml.load(compose_preseed(node))
+        preseed = yaml.safe_load(compose_preseed(node))
         self.assertEqual(
             get_metadata_server_url(),
             preseed['datasource']['MAAS']['metadata_url'])
@@ -126,7 +126,7 @@
 
     def test_compose_preseed_for_commissioning_includes_auth_token(self):
         node = factory.make_node(status=NODE_STATUS.COMMISSIONING)
-        preseed = yaml.load(compose_preseed(node))
+        preseed = yaml.safe_load(compose_preseed(node))
         maas_dict = preseed['datasource']['MAAS']
         token = NodeKey.objects.get_token_for_node(node)
         self.assertEqual(token.consumer.key, maas_dict['consumer_key'])

=== modified file 'src/provisioningserver/plugin.py'
--- src/provisioningserver/plugin.py	2012-04-16 10:01:30 +0000
+++ src/provisioningserver/plugin.py	2012-06-22 09:54:20 +0000
@@ -158,7 +158,7 @@
     @classmethod
     def parse(cls, stream):
         """Load a YAML configuration from `stream` and validate."""
-        return cls.to_python(yaml.load(stream))
+        return cls.to_python(yaml.safe_load(stream))
 
     @classmethod
     def load(cls, filename):

=== modified file 'src/provisioningserver/tests/test_plugin.py'
--- src/provisioningserver/tests/test_plugin.py	2012-04-20 12:54:26 +0000
+++ src/provisioningserver/tests/test_plugin.py	2012-06-22 09:54:20 +0000
@@ -158,7 +158,7 @@
         config.setdefault("password", factory.getRandomString())
         config_filename = os.path.join(self.tempdir, "config.yaml")
         with open(config_filename, "wb") as stream:
-            yaml.dump(config, stream)
+            yaml.safe_dump(config, stream)
         return config_filename
 
     def test_init(self):