> What would that mean for this PR? Should I leave it as is? Or should I > change it so that only the nocloud data source uses the new function? > I don't mind this waiting for the next release, I do understand it is > a slight change in behavior.
In case you missed, I had some comments you can see below in the diff output about the unittests. Let's address those first. As for this branch; it appears to me to be a super-set of capabilities more than a change in existing behavior. That is, I expect the datasource given the same inputs to produce the same behavior; We are likely to take the updated behavior into master, and include that in the Ubuntu E release; it could be disabled by default on previous releases but allowing users to opt-in to the new behavior. Diff comments: > diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py > index 0e71db8..a31031a 100644 > --- a/tests/unittests/test_util.py > +++ b/tests/unittests/test_util.py > @@ -724,21 +724,52 @@ class TestMessageFromString(helpers.TestCase): > self.assertNotIn('\x00', roundtripped) > > > -class TestReadSeeded(helpers.TestCase): > +class TestLoadSeed(helpers.TestCase): > def setUp(self): > - super(TestReadSeeded, self).setUp() > + super(TestLoadSeed, self).setUp() > self.tmp = tempfile.mkdtemp() > self.addCleanup(shutil.rmtree, self.tmp) > > - def test_unicode_not_messed_up(self): > + def test_simple_seed(self): Let's keep the test name here; it's still verifying unicode user-data. And then I think we want to add a scenario that tests a URL as well, right? > ud = b"userdatablob" > helpers.populate_dir( > self.tmp, {'meta-data': "key1: val1", 'user-data': ud}) > - sdir = self.tmp + os.path.sep > - (found_md, found_ud) = util.read_seeded(sdir) > + seed = util.load_seed(self.tmp, required=['meta-data', 'user-data']) > + found_md = seed.get('meta-data', {}) > + found_ud = seed.get('user-data', '') > + > + self.assertEqual(found_md, {'key1': 'val1'}) > + self.assertEqual(found_ud, ud) > + > + def test_ignores_optional(self): > + ud = b"userdatablob" > + helpers.populate_dir( > + self.tmp, {'meta-data': 'key1: val1', 'user-data': ud}) > + seed = util.load_seed( > + self.tmp, ['meta-data', 'user-data'], ['network-config']) > + found_md = seed.get('meta-data', {}) > + found_ud = seed.get('user-data', '') > + > + self.assertEqual(found_md, {'key1': 'val1'}) > + self.assertEqual(found_ud, ud) > + > + def test_loads_optional(self): > + ud = b"userdatablob" > + helpers.populate_dir( > + self.tmp, { > + 'meta-data': 'key1: val1', > + 'user-data': ud, > + 'network-config': 'key: val' > + }) > + seed = util.load_seed( > + self.tmp, ['meta-data', 'user-data'], ['network-config']) > + found_md = seed.get('meta-data', {}) > + found_ud = seed.get('user-data', '') > + found_nc = seed.get('network-config', {}) > > self.assertEqual(found_md, {'key1': 'val1'}) > self.assertEqual(found_ud, ud) > + self.assertEqual(found_nc, {'key': 'val'}) > > > class TestSubp(helpers.CiTestCase): -- https://code.launchpad.net/~bitfehler/cloud-init/+git/cloud-init/+merge/369814 Your team cloud-init commiters is requested to review the proposed merge of ~bitfehler/cloud-init:bitfehler/load_seed into cloud-init:master. _______________________________________________ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp