Review: Needs Information
Diff comments: > === modified file 'cloudinit/config/cc_debug.py' > --- cloudinit/config/cc_debug.py 2014-01-23 19:28:59 +0000 > +++ cloudinit/config/cc_debug.py 2014-10-18 16:32:48 +0000 > @@ -14,10 +14,13 @@ > # You should have received a copy of the GNU General Public License > # along with this program. If not, see <http://www.gnu.org/licenses/>. > > +import copy > +from StringIO import StringIO > + > from cloudinit import type_utils > from cloudinit import util > -import copy > -from StringIO import StringIO > + > +SKIP_KEYS = frozenset(['log_cfgs']) > > > def _make_header(text): > @@ -31,6 +34,11 @@ > return header.getvalue() > > > +def _dumps(obj): > + text = util.yaml_dumps(obj, explicit_start=False, explicit_end=False) > + return text.rstrip() > + > + > def handle(name, cfg, cloud, log, args): > verbose = util.get_cfg_by_path(cfg, ('debug', 'verbose'), default=True) > if args: > @@ -46,7 +54,7 @@ > return > # Clean out some keys that we just don't care about showing... > dump_cfg = copy.deepcopy(cfg) > - for k in ['log_cfgs']: > + for k in SKIP_KEYS: > dump_cfg.pop(k, None) > all_keys = list(dump_cfg.keys()) > for k in all_keys: > @@ -55,10 +63,10 @@ > # Now dump it... > to_print = StringIO() > to_print.write(_make_header("Config")) > - to_print.write(util.yaml_dumps(dump_cfg)) > + to_print.write(_dumps(dump_cfg)) > to_print.write("\n") > to_print.write(_make_header("MetaData")) > - to_print.write(util.yaml_dumps(cloud.datasource.metadata)) > + to_print.write(_dumps(cloud.datasource.metadata)) > to_print.write("\n") > to_print.write(_make_header("Misc")) > to_print.write("Datasource: %s\n" % > > === modified file 'cloudinit/util.py' > --- cloudinit/util.py 2014-09-30 20:24:54 +0000 > +++ cloudinit/util.py 2014-10-18 16:32:48 +0000 > @@ -1270,14 +1270,14 @@ > logexc(LOG, "Failed writing url content to %s", target_fn) > > > -def yaml_dumps(obj): > - formatted = yaml.dump(obj, > - line_break="\n", > - indent=4, > - explicit_start=True, > - explicit_end=True, > - default_flow_style=False) > - return formatted > +def yaml_dumps(obj, explicit_start=True, explicit_end=True): > + return yaml.safe_dump(obj, > + line_break="\n", > + indent=4, > + explicit_start=explicit_start, > + explicit_end=explicit_end, > + default_flow_style=False, > + allow_unicode=True) qq. The last two arguments: default_flow_style and allow_unicode are not being sent as arguments to yaml_dumps. Should it be sending those (with appropriate defaults)? I see that you fixed the other two args - explicit_start and explicit_end. You added those as arguments to yaml_dumps. That seems like the right thing to do. Should we do that for other arguments also? > > > def ensure_dir(path, mode=None): > > === added file 'tests/unittests/test_handler/test_handler_debug.py' > --- tests/unittests/test_handler/test_handler_debug.py 1970-01-01 > 00:00:00 +0000 > +++ tests/unittests/test_handler/test_handler_debug.py 2014-10-18 > 16:32:48 +0000 > @@ -0,0 +1,78 @@ > +# vi: ts=4 expandtab > +# > +# Copyright (C) 2014 Yahoo! Inc. > +# > +# This program is free software: you can redistribute it and/or modify > +# it under the terms of the GNU General Public License version 3, as > +# published by the Free Software Foundation. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > + > +from cloudinit.config import cc_debug > + > +from cloudinit import cloud > +from cloudinit import distros > +from cloudinit import helpers > +from cloudinit import util > + > +from cloudinit.sources import DataSourceNone > + > +from .. import helpers as t_help > + > +import logging > + > +LOG = logging.getLogger(__name__) > + > + > +class TestDebug(t_help.FilesystemMockingTestCase): > + def setUp(self): > + super(TestDebug, self).setUp() > + self.new_root = self.makeDir(prefix="unittest_") > + > + def _get_cloud(self, distro, metadata=None): > + self.patchUtils(self.new_root) > + paths = helpers.Paths({}) > + cls = distros.fetch(distro) > + d = cls(distro, {}, paths) > + ds = DataSourceNone.DataSourceNone({}, d, paths) > + if metadata: > + ds.metadata.update(metadata) > + return cloud.Cloud(ds, paths, {}, d, None) > + > + def test_debug_write(self): > + cfg = { > + 'abc': '123', > + 'c': u'\u20a0', > + 'debug': { > + 'verbose': True, > + # Does not actually write here due to mocking... > + 'output': '/var/log/cloud-init-debug.log', > + }, > + } > + cc = self._get_cloud('ubuntu') > + cc_debug.handle('cc_debug', cfg, cc, LOG, []) > + contents = util.load_file('/var/log/cloud-init-debug.log') > + # Some basic sanity tests... > + self.assertGreater(len(contents), 0) > + for k in cfg.keys(): > + self.assertIn(k, contents) > + > + def test_debug_no_write(self): > + cfg = { > + 'abc': '123', > + 'debug': { > + 'verbose': False, > + # Does not actually write here due to mocking... > + 'output': '/var/log/cloud-init-debug.log', > + }, > + } > + cc = self._get_cloud('ubuntu') > + cc_debug.handle('cc_debug', cfg, cc, LOG, []) > + self.assertRaises(IOError, > + util.load_file, '/var/log/cloud-init-debug.log') > -- https://code.launchpad.net/~harlowja/cloud-init/debug-pretty/+merge/238799 Your team cloud init development team is requested to review the proposed merge of lp:~harlowja/cloud-init/debug-pretty into lp:cloud-init. _______________________________________________ Mailing list: https://launchpad.net/~cloud-init-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
