Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modified scale awareness from exception to warning #1466

Merged
merged 1 commit into from
Jun 4, 2015

Conversation

aanm
Copy link

@aanm aanm commented May 26, 2015

It starts not making sense only use docker-compose on a single host environment. I'm proposing removing the scale awareness of port collision from exception to a warning.
Fixes: #1378
Signed-off-by: André Martins martins@noironetworks.com

@dnephin
Copy link

dnephin commented May 26, 2015

I think this is a good idea. I'll defer to others about the text of the warning.

raise CannotBeScaledError()
log.warn('Service %s only specifies a port on the host. If multiple '
'containers for this service are created on a single host, '
'the port will clash.' % self.name)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty clear warning, but I don't think we need "only".

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you're right, makes more sense without the "only"

Signed-off-by: André Martins <martins@noironetworks.com>
@aanm aanm force-pushed the changing-scale-to-warning branch from 2538283 to ae63d35 Compare May 26, 2015 23:00
@dnephin
Copy link

dnephin commented Jun 4, 2015

LGTM

1 similar comment
@aanand
Copy link

aanand commented Jun 4, 2015

LGTM

aanand added a commit that referenced this pull request Jun 4, 2015
Modified scale awareness from exception to warning
@aanand aanand merged commit 7d2a894 into docker:master Jun 4, 2015
@dnephin dnephin added this to the 1.4.0 milestone Jun 4, 2015
@aanand aanand modified the milestone: 1.4.0 Jun 4, 2015
@chanwit
Copy link

chanwit commented Jun 6, 2015

@aanand @dnephin This is a trivial change, but important. Is it possible to include this change into 1.3? (I know it's a bit late - sorry for that)

@aanand
Copy link

aanand commented Jun 10, 2015

ping @bfirsh - reckon we could consider this a "bug fix" and include it in 1.3?

@bfirsh
Copy link

bfirsh commented Jun 10, 2015

👍

@aanand aanand modified the milestones: 1.3.0, 1.4.0 Jun 10, 2015
@aanand
Copy link

aanand commented Jun 10, 2015

Nice. Let's do another RC with this fix then (after a few days, just in case any more issues crop up).

@chanwit
Copy link

chanwit commented Jun 10, 2015 via email

aanand added a commit to aanand/fig that referenced this pull request Jun 12, 2015
…arning

Modified scale awareness from exception to warning
(cherry picked from commit 7d2a894)

Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
aanand added a commit to aanand/fig that referenced this pull request Jun 15, 2015
…arning

Modified scale awareness from exception to warning
(cherry picked from commit 7d2a894)

Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
@wader
Copy link

wader commented Jun 25, 2015

It should be possible to use scale and ip::containerPort format right? adjust can_be_scaled?

@aanm aanm deleted the changing-scale-to-warning branch December 21, 2015 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow scaling services with static ports on Swarm with affinity
7 participants