Repository: canonical/operator
Pull request #497 (2021-03-28 23:07:51)

Update service config's "environment" to plain dict
The original intention with the list of 1-item objects was so the
environment variables were ordered in case of dependencies, like
`PATH=$PATH:/foo/var` (not that we support interpolation right now).

However, it made for an awkward config format and awkward API,
especially in the Python Operator Framework. So we're going to a plain
dictionary for the environment, and we'll resolve any dependencies
using an explicit dependency check.

This is the Python Operator Framework change to go with the Pebble PR: https://github.com/canonical/pebble/pull/24

See also original issue: https://github.com/canonical/pebble/issues/23


Patch:
diff --git a/ops/pebble.py b/ops/pebble.py
index 9ecfe98..0d00049 100644
--- a/ops/pebble.py
+++ b/ops/pebble.py
@@ -447,12 +447,7 @@ class Layer:
 
 
 class Service:
-    """Represents a service description in a Pebble configuration layer.
-
-    The "environment" attribute is parsed as a list of (key, value) tuples,
-    because that seems most natural for ordered keys and values in Python.
-    In the YAML, however, it's a list of 1-item {key: value} objects.
-    """
+    """Represents a service description in a Pebble configuration layer."""
 
     def __init__(self, name: str, raw: Dict = None):
         self.name = name
@@ -465,18 +460,7 @@ class Service:
         self.after = list(raw.get('after', []))
         self.before = list(raw.get('before', []))
         self.requires = list(raw.get('requires', []))
-        self.environment = self._dicts_to_tuples(raw.get('environment', []))
-
-    @staticmethod
-    def _dicts_to_tuples(dicts):
-        """Convert list of 1-item {k: v} dicts to list of (k, v) tuples."""
-        tuples = []
-        for d in dicts:
-            if len(d) != 1:
-                raise ValueError('expected 1-item dict, got {!r}'.format(d))
-            kv = list(d.items())[0]
-            tuples.append(kv)
-        return tuples
+        self.environment = dict(raw.get('environment', {}))
 
     def to_dict(self) -> Dict:
         """Convert this service object to its dict representation."""
@@ -489,7 +473,7 @@ class Service:
             ('after', self.after),
             ('before', self.before),
             ('requires', self.requires),
-            ('environment', [{k: v} for k, v in self.environment]),
+            ('environment', self.environment),
         ]
         return {name: value for name, value in fields if value}
 


Test patch:
diff --git a/test/test_pebble.py b/test/test_pebble.py
index 4d6dddb..9a0c1a2 100644
--- a/test/test_pebble.py
+++ b/test/test_pebble.py
@@ -419,8 +419,8 @@ services:
   bar:
     command: echo bar
     environment:
-    - ENV1: value1
-    - ENV2: value2
+      ENV1: value1
+      ENV2: value2
     summary: Bar
   foo:
     command: echo foo
@@ -437,7 +437,7 @@ summary: Sum Mary
         self.assertEqual(s.services['bar'].summary, 'Bar')
         self.assertEqual(s.services['bar'].command, 'echo bar')
         self.assertEqual(s.services['bar'].environment,
-                         [('ENV1', 'value1'), ('ENV2', 'value2')])
+                         {'ENV1': 'value1', 'ENV2': 'value2'})
 
         self.assertEqual(s.to_yaml(), yaml)
         self.assertEqual(str(s), yaml)
@@ -454,7 +454,7 @@ class TestService(unittest.TestCase):
         self.assertEqual(service.after, [])
         self.assertEqual(service.before, [])
         self.assertEqual(service.requires, [])
-        self.assertEqual(service.environment, [])
+        self.assertEqual(service.environment, {})
         self.assertEqual(service.to_dict(), {})
 
     def test_name_only(self):
@@ -474,7 +474,7 @@ class TestService(unittest.TestCase):
             'after': ['a1', 'a2'],
             'before': ['b1', 'b2'],
             'requires': ['r1', 'r2'],
-            'environment': [{'k1': 'v1'}, {'k2': 'v2'}],
+            'environment': {'k1': 'v1', 'k2': 'v2'},
         }
         s = pebble.Service('Name 2', d)
         self.assertEqual(s.name, 'Name 2')
@@ -485,7 +485,7 @@ class TestService(unittest.TestCase):
         self.assertEqual(s.after, ['a1', 'a2'])
         self.assertEqual(s.before, ['b1', 'b2'])
         self.assertEqual(s.requires, ['r1', 'r2'])
-        self.assertEqual(s.environment, [('k1', 'v1'), ('k2', 'v2')])
+        self.assertEqual(s.environment, {'k1': 'v1', 'k2': 'v2'})
 
         self.assertEqual(s.to_dict(), d)
 
@@ -493,15 +493,15 @@ class TestService(unittest.TestCase):
         s.after.append('a3')
         s.before.append('b3')
         s.requires.append('r3')
-        s.environment.append(('k3', 'v3'))
+        s.environment['k3'] = 'v3'
         self.assertEqual(s.after, ['a1', 'a2', 'a3'])
         self.assertEqual(s.before, ['b1', 'b2', 'b3'])
         self.assertEqual(s.requires, ['r1', 'r2', 'r3'])
-        self.assertEqual(s.environment, [('k1', 'v1'), ('k2', 'v2'), ('k3', 'v3')])
+        self.assertEqual(s.environment, {'k1': 'v1', 'k2': 'v2', 'k3': 'v3'})
         self.assertEqual(d['after'], ['a1', 'a2'])
         self.assertEqual(d['before'], ['b1', 'b2'])
         self.assertEqual(d['requires'], ['r1', 'r2'])
-        self.assertEqual(d['environment'], [{'k1': 'v1'}, {'k2': 'v2'}])
+        self.assertEqual(d['environment'], {'k1': 'v1', 'k2': 'v2'})
 
 
 class TestServiceInfo(unittest.TestCase):