Skip to content

Commit

Permalink
Address old TODO, and small refactor of container name logic in service.
Browse files Browse the repository at this point in the history
Signed-off-by: Daniel Nephin <[email protected]>
  • Loading branch information
dnephin committed Feb 25, 2016
1 parent cdda616 commit 43ecf87
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 11 deletions.
19 changes: 10 additions & 9 deletions compose/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,11 @@ def scale(self, desired_num, timeout=DEFAULT_TIMEOUT):
- starts containers until there are at least `desired_num` running
- removes all stopped containers
"""
if self.custom_container_name() and desired_num > 1:
if self.custom_container_name and desired_num > 1:
log.warn('The "%s" service is using the custom container name "%s". '
'Docker requires each container to have a unique name. '
'Remove the custom name to scale the service.'
% (self.name, self.custom_container_name()))
% (self.name, self.custom_container_name))

if self.specifies_host_port():
log.warn('The "%s" service specifies a port on the host. If multiple containers '
Expand Down Expand Up @@ -496,10 +496,6 @@ def get_link_names(self):
def get_volumes_from_names(self):
return [s.source.name for s in self.volumes_from if isinstance(s.source, Service)]

def get_container_name(self, number, one_off=False):
# TODO: Implement issue #652 here
return build_container_name(self.project, self.name, number, one_off)

# TODO: this would benefit from github.com/docker/docker/pull/14699
# to remove the need to inspect every container
def _next_container_number(self, one_off=False):
Expand Down Expand Up @@ -561,9 +557,7 @@ def _get_container_create_options(
for k in DOCKER_CONFIG_KEYS if k in self.options)
container_options.update(override_options)

if self.custom_container_name() and not one_off:
container_options['name'] = self.custom_container_name()
elif not container_options.get('name'):
if not container_options.get('name'):
container_options['name'] = self.get_container_name(number, one_off)

container_options.setdefault('detach', True)
Expand Down Expand Up @@ -706,9 +700,16 @@ def labels(self, one_off=False):
'{0}={1}'.format(LABEL_ONE_OFF, "True" if one_off else "False")
]

@property
def custom_container_name(self):
return self.options.get('container_name')

def get_container_name(self, number, one_off=False):
if self.custom_container_name and not one_off:
return self.custom_container_name

return build_container_name(self.project, self.name, number, one_off)

def remove_image(self, image_type):
if not image_type or image_type == ImageType.none:
return False
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/service_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,7 @@ def test_scale_with_custom_container_name_outputs_warning(self, mock_log):
results in warning output.
"""
service = self.create_service('app', container_name='custom-container')
self.assertEqual(service.custom_container_name(), 'custom-container')
self.assertEqual(service.custom_container_name, 'custom-container')

service.scale(3)

Expand Down Expand Up @@ -963,7 +963,7 @@ def test_stop_signal(self):

def test_custom_container_name(self):
service = self.create_service('web', container_name='my-web-container')
self.assertEqual(service.custom_container_name(), 'my-web-container')
self.assertEqual(service.custom_container_name, 'my-web-container')

container = create_and_start_container(service)
self.assertEqual(container.name, 'my-web-container')
Expand Down

0 comments on commit 43ecf87

Please sign in to comment.