Merge pull request #2662 from dnephin/fix_config_printing_unicode

Fix config printing unicode objects

compose/cli/main.py

@@ -9,7 +9,6 @@ import sys
 from inspect import getdoc
 from operator import attrgetter
 
-import yaml
 from docker.errors import APIError
 from requests.exceptions import ReadTimeout
 
@@ -18,6 +17,7 @@ from .. import __version__
 from ..config import config
 from ..config import ConfigurationError
 from ..config import parse_environment
+from ..config.serialize import serialize_config
 from ..const import DEFAULT_TIMEOUT
 from ..const import HTTP_TIMEOUT
 from ..const import IS_WINDOWS_PLATFORM
@@ -215,13 +215,7 @@ class TopLevelCommand(DocoptCommand):
             print('\n'.join(service['name'] for service in compose_config.services))
             return
 
-        compose_config = dict(
-            (service.pop('name'), service) for service in compose_config.services)
-        print(yaml.dump(
-            compose_config,
-            default_flow_style=False,
-            indent=2,
-            width=80))
+        print(serialize_config(compose_config))
 
     def create(self, project, options):
         """

compose/config/serialize.py

@@ -0,0 +1,30 @@
+from __future__ import absolute_import
+from __future__ import unicode_literals
+
+import six
+import yaml
+
+from compose.config import types
+
+
+def serialize_config_type(dumper, data):
+    representer = dumper.represent_str if six.PY3 else dumper.represent_unicode
+    return representer(data.repr())
+
+
+yaml.SafeDumper.add_representer(types.VolumeFromSpec, serialize_config_type)
+yaml.SafeDumper.add_representer(types.VolumeSpec, serialize_config_type)
+
+
+def serialize_config(config):
+    output = {
+        'version': config.version,
+        'services': {service.pop('name'): service for service in config.services},
+        'networks': config.networks,
+        'volumes': config.volumes,
+    }
+    return yaml.safe_dump(
+        output,
+        default_flow_style=False,
+        indent=2,
+        width=80)

compose/config/types.py

@@ -67,6 +67,9 @@ class VolumeFromSpec(namedtuple('_VolumeFromSpec', 'source mode type')):
 
         return cls(source, mode, type)
 
+    def repr(self):
+        return '{v.type}:{v.source}:{v.mode}'.format(v=self)
+
 
 def parse_restart_spec(restart_config):
     if not restart_config:
@@ -156,3 +159,7 @@ class VolumeSpec(namedtuple('_VolumeSpec', 'external internal mode')):
             mode = parts[2]
 
         return cls(external, internal, mode)
+
+    def repr(self):
+        external = self.external + ':' if self.external else ''
+        return '{ext}{v.internal}:{v.mode}'.format(ext=external, v=self)

compose/network.py

@@ -65,7 +65,10 @@ class Network(object):
             )
 
     def remove(self):
-        # TODO: don't remove external networks
+        if self.external_name:
+            log.info("Network %s is external, skipping", self.full_name)
+            return
+
         log.info("Removing network {}".format(self.full_name))
         self.client.remove_network(self.full_name)
 

compose/project.py

@@ -275,7 +275,7 @@ class Project(object):
     def down(self, remove_image_type, include_volumes):
         self.stop()
         self.remove_stopped(v=include_volumes)
-        self.remove_default_network()
+        self.remove_networks()
 
         if include_volumes:
             self.remove_volumes()
@@ -286,11 +286,11 @@ class Project(object):
         for service in self.get_services():
             service.remove_image(remove_image_type)
 
-    def remove_default_network(self):
+    def remove_networks(self):
         if not self.use_networking:
             return
-        if self.uses_default_network():
-            self.default_network.remove()
+        for network in self.networks:
+            network.remove()
 
     def remove_volumes(self):
         for volume in self.volumes:

compose/service.py

@@ -460,7 +460,8 @@ class Service(object):
             'links': self.get_link_names(),
             'net': self.net.id,
             'volumes_from': [
-                (v.source.name, v.mode) for v in self.volumes_from if isinstance(v.source, Service)
+                (v.source.name, v.mode)
+                for v in self.volumes_from if isinstance(v.source, Service)
             ],
         }
 
@@ -519,12 +520,7 @@ class Service(object):
         return links
 
     def _get_volumes_from(self):
-        volumes_from = []
-        for volume_from_spec in self.volumes_from:
-            volumes = build_volume_from(volume_from_spec)
-            volumes_from.extend(volumes)
-
-        return volumes_from
+        return [build_volume_from(spec) for spec in self.volumes_from]
 
     def _get_container_create_options(
             self,
@@ -927,7 +923,7 @@ def warn_on_masked_volume(volumes_option, container_volumes, service):
 
 
 def build_volume_binding(volume_spec):
-    return volume_spec.internal, "{}:{}:{}".format(*volume_spec)
+    return volume_spec.internal, volume_spec.repr()
 
 
 def build_volume_from(volume_from_spec):
@@ -938,12 +934,14 @@ def build_volume_from(volume_from_spec):
     if isinstance(volume_from_spec.source, Service):
         containers = volume_from_spec.source.containers(stopped=True)
         if not containers:
-            return ["{}:{}".format(volume_from_spec.source.create_container().id, volume_from_spec.mode)]
+            return "{}:{}".format(
+                volume_from_spec.source.create_container().id,
+                volume_from_spec.mode)
 
         container = containers[0]
-        return ["{}:{}".format(container.id, volume_from_spec.mode)]
+        return "{}:{}".format(container.id, volume_from_spec.mode)
     elif isinstance(volume_from_spec.source, Container):
-        return ["{}:{}".format(volume_from_spec.source.id, volume_from_spec.mode)]
+        return "{}:{}".format(volume_from_spec.source.id, volume_from_spec.mode)
 
 
 # Labels

tests/acceptance/cli_test.py

@@ -10,8 +10,8 @@ import subprocess
 import time
 from collections import namedtuple
 from operator import attrgetter
-from textwrap import dedent
 
+import yaml
 from docker import errors
 
 from .. import mock
@@ -148,8 +148,9 @@ class CLITestCase(DockerClientTestCase):
         self.base_dir = None
 
     def test_config_list_services(self):
+        self.base_dir = 'tests/fixtures/v2-full'
         result = self.dispatch(['config', '--services'])
-        assert set(result.stdout.rstrip().split('\n')) == {'simple', 'another'}
+        assert set(result.stdout.rstrip().split('\n')) == {'web', 'other'}
 
     def test_config_quiet_with_error(self):
         self.base_dir = None
@@ -160,20 +161,36 @@ class CLITestCase(DockerClientTestCase):
         assert "'notaservice' doesn't have any configuration" in result.stderr
 
     def test_config_quiet(self):
+        self.base_dir = 'tests/fixtures/v2-full'
         assert self.dispatch(['config', '-q']).stdout == ''
 
     def test_config_default(self):
+        self.base_dir = 'tests/fixtures/v2-full'
         result = self.dispatch(['config'])
-        assert dedent("""
-            simple:
-              command: top
-              image: busybox:latest
-        """).lstrip() in result.stdout
-        assert dedent("""
-            another:
-              command: top
-              image: busybox:latest
-        """).lstrip() in result.stdout
+        # assert there are no python objects encoded in the output
+        assert '!!' not in result.stdout
+
+        output = yaml.load(result.stdout)
+        expected = {
+            'version': 2,
+            'volumes': {'data': {'driver': 'local'}},
+            'networks': {'front': {}},
+            'services': {
+                'web': {
+                    'build': {
+                        'context': os.path.abspath(self.base_dir),
+                    },
+                    'networks': ['front', 'default'],
+                    'volumes_from': ['service:other:rw'],
+                },
+                'other': {
+                    'image': 'busybox:latest',
+                    'command': 'top',
+                    'volumes': ['/data:rw'],
+                },
+            },
+        }
+        assert output == expected
 
     def test_ps(self):
         self.project.get_service('simple').create_container()
@@ -340,16 +357,20 @@ class CLITestCase(DockerClientTestCase):
         assert '--rmi flag must be' in result.stderr
 
     def test_down(self):
-        self.base_dir = 'tests/fixtures/shutdown'
+        self.base_dir = 'tests/fixtures/v2-full'
         self.dispatch(['up', '-d'])
-        wait_on_condition(ContainerCountCondition(self.project, 1))
+        wait_on_condition(ContainerCountCondition(self.project, 2))
 
         result = self.dispatch(['down', '--rmi=local', '--volumes'])
-        assert 'Stopping shutdown_web_1' in result.stderr
-        assert 'Removing shutdown_web_1' in result.stderr
-        assert 'Removing volume shutdown_data' in result.stderr
-        assert 'Removing image shutdown_web' in result.stderr
-        assert 'Removing network shutdown_default' in result.stderr
+        assert 'Stopping v2full_web_1' in result.stderr
+        assert 'Stopping v2full_other_1' in result.stderr
+        assert 'Removing v2full_web_1' in result.stderr
+        assert 'Removing v2full_other_1' in result.stderr
+        assert 'Removing volume v2full_data' in result.stderr
+        assert 'Removing image v2full_web' in result.stderr
+        assert 'Removing image busybox' not in result.stderr
+        assert 'Removing network v2full_default' in result.stderr
+        assert 'Removing network v2full_front' in result.stderr
 
     def test_up_detached(self):
         self.dispatch(['up', '-d'])

tests/fixtures/shutdown/docker-compose.yml

@@ -1,10 +0,0 @@
-
-version: 2
-
-volumes:
-  data:
-    driver: local
-
-services:
-  web:
-    build: .

tests/fixtures/v2-full/docker-compose.yml

@@ -0,0 +1,24 @@
+
+version: 2
+
+volumes:
+  data:
+    driver: local
+
+networks:
+  front: {}
+
+services:
+  web:
+    build: .
+    networks:
+      - front
+      - default
+    volumes_from:
+      - other
+
+  other:
+    image: busybox:latest
+    command: top
+    volumes:
+      - /data