Skip to content

Conversation

@chernjie
Copy link

@chernjie chernjie changed the title Addhosts --add-hosts Mar 23, 2015
@chernjie chernjie force-pushed the addhosts branch 3 times, most recently from 1f61beb to 72f4e02 Compare March 24, 2015 01:10
@chernjie
Copy link
Author

@tdesvenain @sampwing I have signed your commits after rebasing from the latest master.

@tdesvenain
Copy link

Great, thanks !

@chernjie
Copy link
Author

I am wondering if it is possible to add an alias to extra_hosts, e.g. hosts

hosts:
 - docker: 162.242.195.82
 - fig: 50.31.209.229

Thoughts?

@chernjie chernjie changed the title --add-hosts Add extra_hosts to yml configuration --add-hosts Mar 24, 2015
@pedrokiefer
Copy link

Shouldn't it be possible to add and extra_host that is another container, but not actually linked to the current container.

Something like:

web:
  build: .
worker:
  build: .
  extra_hosts:
     - web

@aanand
Copy link

aanand commented Mar 27, 2015

@pedrokiefer Why not just use a link? You'll get the /etc/hosts entry.

@chernjie
Copy link
Author

@pedrokiefer : What @aanand said, use link for containers, use extra_hosts for non-containers

@pedrokiefer
Copy link

I had some kind of circular dependency between containers, so I thought extra_hosts would solve that to me.

@chernjie chernjie force-pushed the addhosts branch 3 times, most recently from 7d0ac5b to 7bf8f3a Compare March 28, 2015 19:21
@chernjie
Copy link
Author

rebased with CI fixes

@chernjie chernjie force-pushed the addhosts branch 10 times, most recently from 76f5fcb to b68130a Compare March 30, 2015 08:02
@chernjie
Copy link
Author

@aanand I would appreciate your feedback/input with this PR. The integration tests are working well. Is there anything that I have missed?

Copy link

Choose a reason for hiding this comment

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

basestring should be six.string_types() for Python 3 compatibility

@chernjie
Copy link
Author

Added .sort instead of set, would this work?
cc: @aanand

@chernjie
Copy link
Author

@aanand any updates to this PR? Any feedback would be appreciated

@aanand
Copy link

aanand commented Apr 21, 2015

set(...) is preferable for consistency with other tests.

…he --add-host flag from the docker client

Signed-off-by: Sam Wing <sampwing@gmail.com>
@chernjie
Copy link
Author

@aanand Updated with set(...)

@aanand
Copy link

aanand commented Apr 23, 2015

Fantastic. Could you squash and rebase? Thanks.

tdesvenain and others added 2 commits April 24, 2015 09:21
Added unit tests in build_extra_hosts + fix

Signed-off-by: CJ <lim@chernjie.com>
linting...
six.string_types
list-of-strings in examples
disallow extra_hosts support for list-of-dicts
A more thorough sets of tests for extra_hosts
Provide better examples
As per @aanand's [comment](https://github.com/docker/compose/pull/1158/files#r28326312)

  I think it'd be better to check `if not isinstance(extra_hosts_line,
  six.string_types)` and raise an error saying `extra_hosts_config must be
  either a list of strings or a string->string mapping`. We shouldn't need
  to do anything special with the list-of-dicts case.
order result to work with assert
use set() instead of sort()

Signed-off-by: CJ <lim@chernjie.com>
@chernjie
Copy link
Author

@aanand I have rebased and squashed all my commits. I have left the other author's contribution as individual commit so as not to lose their contribution information. I hope that's okay

Copy link

Choose a reason for hiding this comment

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

maybe 'extra_host': 'extra_hosts', as well ?

Copy link
Author

Choose a reason for hiding this comment

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

Added @ 86a08c0

@dnephin
Copy link

dnephin commented Apr 24, 2015

I like this, I think it addresses the "external" type described in #988

LGTM

Signed-off-by: CJ <lim@chernjie.com>
@aanand
Copy link

aanand commented Apr 27, 2015

LGTM

aanand added a commit that referenced this pull request Apr 27, 2015
Add extra_hosts to yml configuration --add-hosts
@aanand aanand merged commit 16f8106 into docker:master Apr 27, 2015
@aanand aanand added this to the 1.3.0 milestone Apr 27, 2015
@rysiekpl
Copy link

Oh this is sweet, and I'm kind of craving this feature right now. Any date set for 1.3.0 release?

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.

8 participants