Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
I'm happy with the way it is here... we got rid of all 'import yaml' from cloudinit.util at least. and down to a signle place that 'yaml.load' is called. -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage 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
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
Review: Approve continuous-integration PASSED: Continuous integration, rev:e69df5dce264f9f329dd0630aad3954b32999153 https://jenkins.ubuntu.com/server/job/cloud-init-ci/1234/ Executed test runs: SUCCESS: Checkout SUCCESS: Unit & Style Tests SUCCESS: Ubuntu LTS: Build SUCCESS: Ubuntu LTS: Integration IN_PROGRESS: Declarative: Post Actions Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1234//rebuild -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage 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
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:e69df5dce264f9f329dd0630aad3954b32999153 https://jenkins.ubuntu.com/server/job/cloud-init-ci/1233/ Executed test runs: FAILED: Checkout Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1233//rebuild -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage 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
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
OK. I just wanted to make sure that I wasn't missing something on the dump path. For relocating; there are quite a few yaml users inside util.py so we'd still need to import safeyaml into util; so module load wise, we're still pulling it in on any import of util. additionally, moving the load_yaml into safeyaml would mean inter-module deps; load_yaml uses decode_binary(), so we'd need to duplicate that. I'm fine with this as it is, but would also review moving all yaml operations into safeyaml. -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage 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
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
Review: Approve continuous-integration PASSED: Continuous integration, rev:e7c84bfdc42bc533a8f737ecd6b9557b466e1ebd https://jenkins.ubuntu.com/server/job/cloud-init-ci/1232/ Executed test runs: SUCCESS: Checkout SUCCESS: Unit & Style Tests SUCCESS: Ubuntu LTS: Build SUCCESS: Ubuntu LTS: Integration IN_PROGRESS: Declarative: Post Actions Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1232//rebuild -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage 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
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
not a real advantage other than getting closer to getting all yaml usage into a single place. On Thu, Oct 24, 2019 at 12:56 PM Ryan Harper wrote: > > Is there an advantage to moving dumps into safeyaml? I didn't think dump had > any concerns. Could we just have replaced the yaml.load() calls with > util.load_yaml() ? > -- > https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 > You are the owner of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage. -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage 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
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
just in better use of modules... putting yaml things into their own module rather than having them all in util. I'm willing to move load_yaml also if you'd like. which would mean util would have no yaml code in it. -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage 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
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
Is there an advantage to moving dumps into safeyaml? I didn't think dump had any concerns. Could we just have replaced the yaml.load() calls with util.load_yaml() ? -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679 Your team cloud-init Commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1849640-adjust-yaml-usage 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
[Cloud-init-dev] [Merge] ~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master
Scott Moser has proposed merging
~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master.
Commit message:
Fix usages of yaml, and move yaml_dump to safeyaml.dumps.
Here we replace uses of the pyyaml module directly with functions
provided by cloudinit.safeyaml. Also, change/move
cloudinit.util.yaml_dumps
to
cloudinit.safeyaml.dumps
LP: #1849640
Requested reviews:
cloud-init Commiters (cloud-init-dev)
For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/374679
see commit message
--
Your team cloud-init Commiters is requested to review the proposed merge of
~smoser/cloud-init:fix/1849640-adjust-yaml-usage into cloud-init:master.
diff --git a/cloudinit/cmd/devel/net_convert.py b/cloudinit/cmd/devel/net_convert.py
index 1ad7e0b..9b76830 100755
--- a/cloudinit/cmd/devel/net_convert.py
+++ b/cloudinit/cmd/devel/net_convert.py
@@ -5,13 +5,12 @@ import argparse
import json
import os
import sys
-import yaml
from cloudinit.sources.helpers import openstack
from cloudinit.sources import DataSourceAzure as azure
from cloudinit.sources import DataSourceOVF as ovf
-from cloudinit import distros
+from cloudinit import distros, safeyaml
from cloudinit.net import eni, netplan, network_state, sysconfig
from cloudinit import log
@@ -78,13 +77,12 @@ def handle_args(name, args):
if args.kind == "eni":
pre_ns = eni.convert_eni_data(net_data)
elif args.kind == "yaml":
-pre_ns = yaml.load(net_data)
+pre_ns = safeyaml.load(net_data)
if 'network' in pre_ns:
pre_ns = pre_ns.get('network')
if args.debug:
sys.stderr.write('\n'.join(
-["Input YAML",
- yaml.dump(pre_ns, default_flow_style=False, indent=4), ""]))
+["Input YAML", safeyaml.dumps(pre_ns), ""]))
elif args.kind == 'network_data.json':
pre_ns = openstack.convert_net_json(
json.loads(net_data), known_macs=known_macs)
@@ -100,9 +98,8 @@ def handle_args(name, args):
"input data")
if args.debug:
-sys.stderr.write('\n'.join([
-"", "Internal State",
-yaml.dump(ns, default_flow_style=False, indent=4), ""]))
+sys.stderr.write('\n'.join(
+["", "Internal State", safeyaml.dumps(ns), ""]))
distro_cls = distros.fetch(args.distro)
distro = distro_cls(args.distro, {}, None)
config = {}
diff --git a/cloudinit/cmd/tests/test_main.py b/cloudinit/cmd/tests/test_main.py
index a1e534f..57b8fdf 100644
--- a/cloudinit/cmd/tests/test_main.py
+++ b/cloudinit/cmd/tests/test_main.py
@@ -6,8 +6,9 @@ import os
from six import StringIO
from cloudinit.cmd import main
+from cloudinit import safeyaml
from cloudinit.util import (
-ensure_dir, load_file, write_file, yaml_dumps)
+ensure_dir, load_file, write_file)
from cloudinit.tests.helpers import (
FilesystemMockingTestCase, wrap_and_call)
@@ -39,7 +40,7 @@ class TestMain(FilesystemMockingTestCase):
],
'cloud_init_modules': ['write-files', 'runcmd'],
}
-cloud_cfg = yaml_dumps(self.cfg)
+cloud_cfg = safeyaml.dumps(self.cfg)
ensure_dir(os.path.join(self.new_root, 'etc', 'cloud'))
self.cloud_cfg_file = os.path.join(
self.new_root, 'etc', 'cloud', 'cloud.cfg')
@@ -113,7 +114,7 @@ class TestMain(FilesystemMockingTestCase):
"""When local-hostname metadata is present, call cc_set_hostname."""
self.cfg['datasource'] = {
'None': {'metadata': {'local-hostname': 'md-hostname'}}}
-cloud_cfg = yaml_dumps(self.cfg)
+cloud_cfg = safeyaml.dumps(self.cfg)
write_file(self.cloud_cfg_file, cloud_cfg)
cmdargs = myargs(
debug=False, files=None, force=False, local=False, reporter=None,
diff --git a/cloudinit/config/cc_debug.py b/cloudinit/config/cc_debug.py
index 0a039eb..610dbc8 100644
--- a/cloudinit/config/cc_debug.py
+++ b/cloudinit/config/cc_debug.py
@@ -33,6 +33,7 @@ from six import StringIO
from cloudinit import type_utils
from cloudinit import util
+from cloudinit import safeyaml
SKIP_KEYS = frozenset(['log_cfgs'])
@@ -49,7 +50,7 @@ def _make_header(text):
def _dumps(obj):
-text = util.yaml_dumps(obj, explicit_start=False, explicit_end=False)
+text = safeyaml.dumps(obj, explicit_start=False, explicit_end=False)
return text.rstrip()
diff --git a/cloudinit/config/cc_salt_minion.py b/cloudinit/config/cc_salt_minion.py
index d6a21d7..cd9cb0b 100644
--- a/cloudinit/config/cc_salt_minion.py
+++ b/cloudinit/config/cc_salt_minion.py
@@ -45,7 +45,7 @@ specify them with ``pkg_name``, ``service_name`` and ``config_dir``.
import os
-from cloudinit import util
+from cloudinit import safeyaml, util
# Note: see https://docs.saltstack.com/en/latest/topics/installation/
# Note: see https://docs.saltstack.com/en/latest/ref/configuration/
@@
