lmenezes / cerebro-docker

official cerebro docker image
118 stars 40 forks source link

Support CEREBRO_PORT environment variable #5

Closed matthew-gill closed 4 years ago

matthew-gill commented 4 years ago

As per my issue here, I'd like to be able to change the cerebro port at runtime.

This would allow the use of cerebro within a Kubernetes cluster without port clashes. This is quite common for docker containers.

I've made some changes to the Dockerfile to swap to using an entrypoint.sh file which reads from the environment variable CEREBRO_PORT - this will default to 9000 if not specified (so it remains compatible).

I don't see any compatibility breakages here, my slight concern is dropping the EXPOSE command in the Dockerfile - but as this is optional (and the existing instructions get the user to specify this manually), I believe it's superfluous.

moliware commented 4 years ago

Hi!

Thanks for contributing! Now I fully understand #4

The changes looks good but I feel that the right thing to do is to start using CEREBRO_PORT in the configuration of the main project (https://github.com/lmenezes/cerebro). My plan is to make a release tomorrow, so I will add the possibility of configuring port via env var (actually newer version of play already have onePLAY_HTTP_PORT).

Once I do this I'll edit a bit your PR and merge.

Thanks again!

matthew-gill commented 4 years ago

That's great news! Thank you :)