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

DOCKER_HOST_IP Enironment Variable #180

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

hmusavi
Copy link

@hmusavi hmusavi commented Apr 21, 2017

Added DOCKER_HOST_IP environment variable to docker_compose file to avoid hard-coding them and not require changes to yml file for user environment. Also added two convenience scripts to the kafka image.

wurstmeister and others added 3 commits February 14, 2017 18:23
…he IP address the files. Also added two shell scripts to make it easier to start producers and consumers inside kafka container.
@sscaling
Copy link
Collaborator

sscaling commented Mar 2, 2018

Hi @hmusavi, thanks for the PR. Could you provide a bit more context on the issue this is solving and perhaps the use cases that it addresses. I'm just trying to get my head around this change.

The docker-compose.yml files are examples and, in general, should be configured per environment. Typically you'd have a kubernetes/marathon/swarm configuration files for a specific deployment.

# Conflicts:
#	Dockerfile
#	README.md
#	broker-list.sh
@hmusavi
Copy link
Author

hmusavi commented Mar 12, 2018

My goal was to make the whole configuration independent of the user environment so one can use it out of the box without having to modify the docker-compose or anything. All one has to do is define the right environment variable (or a .env) file and use the image straight from DockerHub even. For instance, instead of having the local ip address of the host in the docker-compose, if we have use an environment variable, it achieves that goal. Then I ended up using the same variable in a couple of scripts as well. Now that we have it, why not.

Additionally, I just added the two kafka console scripts to the docker image as well because I found them quite handy having them around in the image. I use them all the time for troubleshooting and figured other may find them useful too.

So, as you can see, these are very trivial changes that I have deployed in my fork. If you approve and merge them, then I won't have to keep a separate fork. I can use your image out of the box. But if you feel that these aren't necessary, I understand ; feel free to ignore them. It won't hurt my feeling :-)

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.

3 participants