Move service sorting to config package.

Signed-off-by: Daniel Nephin <dnephin@docker.com>

compose/config/__init__.py

@@ -2,7 +2,6 @@
 from .config import ConfigurationError
 from .config import DOCKER_CONFIG_KEYS
 from .config import find
-from .config import get_service_name_from_net
 from .config import load
 from .config import merge_environment
 from .config import parse_environment

compose/config/config.py

@@ -14,6 +14,8 @@ from .errors import CircularReference
 from .errors import ComposeFileNotFound
 from .errors import ConfigurationError
 from .interpolation import interpolate_environment_variables
+from .sort_services import get_service_name_from_net
+from .sort_services import sort_service_dicts
 from .types import parse_extra_hosts
 from .types import parse_restart_spec
 from .types import VolumeFromSpec
@@ -214,10 +216,10 @@ def load(config_details):
         return service_dict
 
     def build_services(config_file):
-        return [
+        return sort_service_dicts([
             build_service(config_file.filename, name, service_dict)
             for name, service_dict in config_file.config.items()
-        ]
+        ])
 
     def merge_services(base, override):
         all_service_names = set(base) | set(override)
@@ -638,17 +640,6 @@ def to_list(value):
         return value
 
 
-def get_service_name_from_net(net_config):
-    if not net_config:
-        return
-
-    if not net_config.startswith('container:'):
-        return
-
-    _, net_name = net_config.split(':', 1)
-    return net_name
-
-
 def load_yaml(filename):
     try:
         with open(filename, 'r') as fh:

compose/config/errors.py

@@ -6,6 +6,10 @@ class ConfigurationError(Exception):
         return self.msg
 
 
+class DependencyError(ConfigurationError):
+    pass
+
+
 class CircularReference(ConfigurationError):
     def __init__(self, trail):
         self.trail = trail

compose/config/sort_services.py

@@ -0,0 +1,55 @@
+from compose.config.errors import DependencyError
+
+
+def get_service_name_from_net(net_config):
+    if not net_config:
+        return
+
+    if not net_config.startswith('container:'):
+        return
+
+    _, net_name = net_config.split(':', 1)
+    return net_name
+
+
+def sort_service_dicts(services):
+    # Topological sort (Cormen/Tarjan algorithm).
+    unmarked = services[:]
+    temporary_marked = set()
+    sorted_services = []
+
+    def get_service_names(links):
+        return [link.split(':')[0] for link in links]
+
+    def get_service_names_from_volumes_from(volumes_from):
+        return [volume_from.source for volume_from in volumes_from]
+
+    def get_service_dependents(service_dict, services):
+        name = service_dict['name']
+        return [
+            service for service in services
+            if (name in get_service_names(service.get('links', [])) or
+                name in get_service_names_from_volumes_from(service.get('volumes_from', [])) or
+                name == get_service_name_from_net(service.get('net')))
+        ]
+
+    def visit(n):
+        if n['name'] in temporary_marked:
+            if n['name'] in get_service_names(n.get('links', [])):
+                raise DependencyError('A service can not link to itself: %s' % n['name'])
+            if n['name'] in n.get('volumes_from', []):
+                raise DependencyError('A service can not mount itself as volume: %s' % n['name'])
+            else:
+                raise DependencyError('Circular import between %s' % ' and '.join(temporary_marked))
+        if n in unmarked:
+            temporary_marked.add(n['name'])
+            for m in get_service_dependents(n, services):
+                visit(m)
+            temporary_marked.remove(n['name'])
+            unmarked.remove(n)
+            sorted_services.insert(0, n)
+
+    while unmarked:
+        visit(unmarked[-1])
+
+    return sorted_services

compose/project.py

@@ -9,7 +9,7 @@ from docker.errors import NotFound
 
 from . import parallel
 from .config import ConfigurationError
-from .config import get_service_name_from_net
+from .config.sort_services import get_service_name_from_net
 from .const import DEFAULT_TIMEOUT
 from .const import LABEL_ONE_OFF
 from .const import LABEL_PROJECT
@@ -26,49 +26,6 @@ from .service import ServiceNet
 log = logging.getLogger(__name__)
 
 
-def sort_service_dicts(services):
-    # Topological sort (Cormen/Tarjan algorithm).
-    unmarked = services[:]
-    temporary_marked = set()
-    sorted_services = []
-
-    def get_service_names(links):
-        return [link.split(':')[0] for link in links]
-
-    def get_service_names_from_volumes_from(volumes_from):
-        return [volume_from.source for volume_from in volumes_from]
-
-    def get_service_dependents(service_dict, services):
-        name = service_dict['name']
-        return [
-            service for service in services
-            if (name in get_service_names(service.get('links', [])) or
-                name in get_service_names_from_volumes_from(service.get('volumes_from', [])) or
-                name == get_service_name_from_net(service.get('net')))
-        ]
-
-    def visit(n):
-        if n['name'] in temporary_marked:
-            if n['name'] in get_service_names(n.get('links', [])):
-                raise DependencyError('A service can not link to itself: %s' % n['name'])
-            if n['name'] in n.get('volumes_from', []):
-                raise DependencyError('A service can not mount itself as volume: %s' % n['name'])
-            else:
-                raise DependencyError('Circular import between %s' % ' and '.join(temporary_marked))
-        if n in unmarked:
-            temporary_marked.add(n['name'])
-            for m in get_service_dependents(n, services):
-                visit(m)
-            temporary_marked.remove(n['name'])
-            unmarked.remove(n)
-            sorted_services.insert(0, n)
-
-    while unmarked:
-        visit(unmarked[-1])
-
-    return sorted_services
-
-
 class Project(object):
     """
     A collection of services.
@@ -96,7 +53,7 @@ class Project(object):
         if use_networking:
             remove_links(service_dicts)
 
-        for service_dict in sort_service_dicts(service_dicts):
+        for service_dict in service_dicts:
             links = project.get_links(service_dict)
             volumes_from = project.get_volumes_from(service_dict)
             net = project.get_net(service_dict)
@@ -404,7 +361,3 @@ class NoSuchService(Exception):
 
     def __str__(self):
         return self.msg
-
-
-class DependencyError(ConfigurationError):
-    pass

tests/integration/testcases.py

@@ -8,6 +8,7 @@ from pytest import skip
 from .. import unittest
 from compose.cli.docker_client import docker_client
 from compose.config.config import resolve_environment
+from compose.config.config import ServiceConfig
 from compose.const import LABEL_PROJECT
 from compose.progress_stream import stream_output
 from compose.service import Service

tests/unit/config/config_test.py

@@ -77,7 +77,7 @@ class ConfigTest(unittest.TestCase):
                 )
             )
 
-    def test_config_invalid_service_names(self):
+    def test_load_config_invalid_service_names(self):
         for invalid_name in ['?not?allowed', ' ', '', '!', '/', '\xe2']:
             with pytest.raises(ConfigurationError) as exc:
                 config.load(build_config_details(
@@ -232,6 +232,27 @@ class ConfigTest(unittest.TestCase):
         assert "service 'bogus' doesn't have any configuration" in exc.exconly()
         assert "In file 'override.yaml'" in exc.exconly()
 
+    def test_load_sorts_in_dependency_order(self):
+        config_details = build_config_details({
+            'web': {
+                'image': 'busybox:latest',
+                'links': ['db'],
+            },
+            'db': {
+                'image': 'busybox:latest',
+                'volumes_from': ['volume:ro']
+            },
+            'volume': {
+                'image': 'busybox:latest',
+                'volumes': ['/tmp'],
+            }
+        })
+        services = config.load(config_details)
+
+        assert services[0]['name'] == 'volume'
+        assert services[1]['name'] == 'db'
+        assert services[2]['name'] == 'web'
+
     def test_config_valid_service_names(self):
         for valid_name in ['_', '-', '.__.', '_what-up.', 'what_.up----', 'whatup']:
             services = config.load(

tests/unit/config/sort_services_test.py

@@ -1,7 +1,7 @@
-from .. import unittest
+from compose.config.errors import DependencyError
+from compose.config.sort_services import sort_service_dicts
 from compose.config.types import VolumeFromSpec
-from compose.project import DependencyError
+from tests import unittest
-from compose.project import sort_service_dicts
 
 
 class SortServiceTest(unittest.TestCase):

tests/unit/project_test.py

@@ -34,29 +34,6 @@ class ProjectTest(unittest.TestCase):
         self.assertEqual(project.get_service('db').name, 'db')
         self.assertEqual(project.get_service('db').options['image'], 'busybox:latest')
 
-    def test_from_dict_sorts_in_dependency_order(self):
-        project = Project.from_dicts('composetest', [
-            {
-                'name': 'web',
-                'image': 'busybox:latest',
-                'links': ['db'],
-            },
-            {
-                'name': 'db',
-                'image': 'busybox:latest',
-                'volumes_from': [VolumeFromSpec('volume', 'ro')]
-            },
-            {
-                'name': 'volume',
-                'image': 'busybox:latest',
-                'volumes': ['/tmp'],
-            }
-        ], None)
-
-        self.assertEqual(project.services[0].name, 'volume')
-        self.assertEqual(project.services[1].name, 'db')
-        self.assertEqual(project.services[2].name, 'web')
-
     def test_from_config(self):
         dicts = [
             {

tests/unit/service_test.py

@@ -23,8 +23,6 @@ from compose.service import NoSuchImageError
 from compose.service import parse_repository_tag
 from compose.service import Service
 from compose.service import ServiceNet
-from compose.service import VolumeFromSpec
-from compose.service import VolumeSpec
 from compose.service import warn_on_masked_volume