Repository: canonical/operator
Pull request #474 (2021-02-09 21:27:18)

Move ops/_yaml.py to ops/_private/yaml.py
Per Mattermost discussion, want to avoid adding individual files to
the top-level ops directory to keep that clean, so add a single _private
directory and we can put this (and any other) private/internal stuff
there.


Patch:
diff --git a/ops/_private/__init__.py b/ops/_private/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/ops/_yaml.py b/ops/_private/yaml.py
similarity index 100%
rename from ops/_yaml.py
rename to ops/_private/yaml.py
diff --git a/ops/charm.py b/ops/charm.py
index 381dac8..2536fe2 100755
--- a/ops/charm.py
+++ b/ops/charm.py
@@ -19,9 +19,9 @@ import os
 import pathlib
 import typing
 
-from ops.framework import Object, EventSource, EventBase, Framework, ObjectEvents
-from ops import _yaml
 from ops import model
+from ops._private import yaml
+from ops.framework import Object, EventSource, EventBase, Framework, ObjectEvents
 
 
 class HookEvent(EventBase):
@@ -697,10 +697,10 @@ class CharmMeta:
                 This can be a simple string, or a file-like object. (passed to `yaml.safe_load`).
             actions: YAML description of Actions for this charm (eg actions.yaml)
         """
-        meta = _yaml.safe_load(metadata)
+        meta = yaml.safe_load(metadata)
         raw_actions = {}
         if actions is not None:
-            raw_actions = _yaml.safe_load(actions)
+            raw_actions = yaml.safe_load(actions)
             if raw_actions is None:
                 raw_actions = {}
         return cls(meta, raw_actions)
diff --git a/ops/model.py b/ops/model.py
index 4e95eba..e295e9c 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -31,10 +31,10 @@ from collections.abc import Mapping, MutableMapping
 from pathlib import Path
 from subprocess import run, PIPE, CalledProcessError
 
+from ops._private import yaml
+from ops.jujuversion import JujuVersion
 import ops
 import ops.pebble as pebble
-from ops.jujuversion import JujuVersion
-from ops import _yaml
 
 
 class Model:
@@ -1236,12 +1236,12 @@ class _ModelBackend:
         try:
             spec_path = tmpdir / 'spec.yaml'
             with spec_path.open("wt", encoding="utf8") as f:
-                _yaml.safe_dump(spec, stream=f)
+                yaml.safe_dump(spec, stream=f)
             args = ['--file', str(spec_path)]
             if k8s_resources:
                 k8s_res_path = tmpdir / 'k8s-resources.yaml'
                 with k8s_res_path.open("wt", encoding="utf8") as f:
-                    _yaml.safe_dump(k8s_resources, stream=f)
+                    yaml.safe_dump(k8s_resources, stream=f)
                 args.extend(['--k8s-resources', str(k8s_res_path)])
             self._run('pod-spec-set', *args)
         finally:
diff --git a/ops/pebble.py b/ops/pebble.py
index f3312a9..d8ea3db 100644
--- a/ops/pebble.py
+++ b/ops/pebble.py
@@ -29,7 +29,7 @@ import urllib.parse
 import urllib.request
 import sys
 
-from ops import _yaml
+from ops._private import yaml
 
 
 _not_provided = object()
@@ -323,7 +323,7 @@ class Layer:
 
     def __init__(self, raw: Union[str, Dict] = None):
         if isinstance(raw, str):
-            d = _yaml.safe_load(raw) or {}
+            d = yaml.safe_load(raw) or {}
         else:
             d = raw or {}
         self.summary = d.get('summary', '')
@@ -333,7 +333,7 @@ class Layer:
 
     def to_yaml(self) -> str:
         """Convert this layer to its YAML representation."""
-        return _yaml.safe_dump(self.to_dict())
+        return yaml.safe_dump(self.to_dict())
 
     def to_dict(self) -> Dict:
         """Convert this layer to its dict representation."""


Test patch:
diff --git a/ops/testing.py b/ops/testing.py
index 65ab761..0bce767 100755
--- a/ops/testing.py
+++ b/ops/testing.py
@@ -23,12 +23,12 @@ from contextlib import contextmanager
 from textwrap import dedent
 
 from ops import (
-    _yaml,
     charm,
     framework,
     model,
     storage,
 )
+from ops._private import yaml
 
 
 # OptionalYAML is something like metadata.yaml or actions.yaml. You can
@@ -273,7 +273,7 @@ class Harness:
                 charm_config = '{}'
         elif isinstance(charm_config, str):
             charm_config = dedent(charm_config)
-        charm_config = _yaml.safe_load(charm_config)
+        charm_config = yaml.safe_load(charm_config)
         charm_config = charm_config.get('options', {})
         return {key: value['default'] for key, value in charm_config.items()
                 if 'default' in value}
@@ -300,7 +300,7 @@ class Harness:
         if self._meta.resources[resource_name].type != "oci-image":
             raise RuntimeError('Resource {} is not an OCI Image'.format(resource_name))
 
-        as_yaml = _yaml.safe_dump(contents)
+        as_yaml = yaml.safe_dump(contents)
         self._backend._resources_map[resource_name] = ('contents.yaml', as_yaml)
 
     def add_resource(self, resource_name: str, content: typing.AnyStr) -> None:
diff --git a/test/test_helpers.py b/test/test_helpers.py
index 2bd06fa..576d560 100755
--- a/test/test_helpers.py
+++ b/test/test_helpers.py
@@ -12,7 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import io
 import os
 import pathlib
 import subprocess
@@ -20,9 +19,6 @@ import shutil
 import tempfile
 import unittest
 
-import yaml
-
-from ops import _yaml
 from ops.framework import Framework
 from ops.model import Model, _ModelBackend
 from ops.charm import CharmMeta
@@ -134,31 +130,3 @@ class BaseTestCase(unittest.TestCase):
         meta = CharmMeta()
         model = Model(meta, backend)
         return model
-
-
-class YAMLTest:
-    pass
-
-
-class TestYAML(unittest.TestCase):
-    def test_safe_load(self):
-        d = _yaml.safe_load('foo: bar\nbaz: 123\n')
-        self.assertEqual(len(d), 2)
-        self.assertEqual(d['foo'], 'bar')
-        self.assertEqual(d['baz'], 123)
-
-        # Should error -- it's not safe to load an instance of a user-defined class
-        with self.assertRaises(yaml.YAMLError):
-            _yaml.safe_load('!!python/object:test.test_helpers.YAMLTest {}')
-
-    def test_safe_dump(self):
-        s = _yaml.safe_dump({'foo': 'bar', 'baz': 123})
-        self.assertEqual(s, 'baz: 123\nfoo: bar\n')
-
-        f = io.StringIO()
-        s = _yaml.safe_dump({'foo': 'bar', 'baz': 123}, stream=f)
-        self.assertEqual(f.getvalue(), 'baz: 123\nfoo: bar\n')
-
-        # Should error -- it's not safe to dump an instance of a user-defined class
-        with self.assertRaises(yaml.YAMLError):
-            _yaml.safe_dump(YAMLTest())
diff --git a/test/test_private.py b/test/test_private.py
new file mode 100644
index 0000000..315875a
--- /dev/null
+++ b/test/test_private.py
@@ -0,0 +1,48 @@
+# Copyright 2021 Canonical Ltd.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import io
+import unittest
+
+import yaml as base_yaml
+
+from ops._private import yaml
+
+
+class YAMLTest:
+    pass
+
+
+class TestYAML(unittest.TestCase):
+    def test_safe_load(self):
+        d = yaml.safe_load('foo: bar\nbaz: 123\n')
+        self.assertEqual(len(d), 2)
+        self.assertEqual(d['foo'], 'bar')
+        self.assertEqual(d['baz'], 123)
+
+        # Should error -- it's not safe to load an instance of a user-defined class
+        with self.assertRaises(base_yaml.YAMLError):
+            yaml.safe_load('!!python/object:test.test_helpers.YAMLTest {}')
+
+    def test_safe_dump(self):
+        s = yaml.safe_dump({'foo': 'bar', 'baz': 123})
+        self.assertEqual(s, 'baz: 123\nfoo: bar\n')
+
+        f = io.StringIO()
+        s = yaml.safe_dump({'foo': 'bar', 'baz': 123}, stream=f)
+        self.assertEqual(f.getvalue(), 'baz: 123\nfoo: bar\n')
+
+        # Should error -- it's not safe to dump an instance of a user-defined class
+        with self.assertRaises(base_yaml.YAMLError):
+            yaml.safe_dump(YAMLTest())