add Dockerfile that allows configuration through environment variables #13
Conversation
kpankonen
force-pushed
the
dockerfile
branch
2 times, most recently
from
a1d00a2 to
17ab687
Compare
|
Updated CircleCI config to push docker image to hub for master and tags. Requires the following environment variables to be set in CircleCI (or it won't attempt the deployment): DOCKER_USERNAME, DOCKER_PASSWORD, DOCKER_REPO |
kpankonen
force-pushed
the
dockerfile
branch
3 times, most recently
from
af774ac to
be03f22
Compare
|
@kpankonen any update on this? I would love if ld-relay was in a container. |
kpankonen
force-pushed
the
dockerfile
branch
4 times, most recently
from
cbbd64b to
6535b8a
Compare
Dockerfile
Outdated
| @@ -0,0 +1,14 @@ | |||
| FROM alpine:3.4 | |||
There was a problem hiding this comment.
FYI Latest Alpine is currently 3.6 - https://hub.docker.com/_/alpine/
docker-entrypoint.sh
Outdated
| streamUri = \"${STREAM_URI:-https://stream.launchdarkly.com}\" | ||
| baseUri = \"${BASE_URI:-https://app.launchdarkly.com}\" | ||
| exitOnError = ${EXIT_ON_ERROR:-false} | ||
| port = ${PORT:-8030} |
There was a problem hiding this comment.
The Dockerfile exposes 8030, yet here the port is configurable. consider simply leaving this non-configurable as you can map the port with docker anyways. Also, PORT is a generic Env name and can clash with existing Vars - consider a more unique name like LD_RELAY_PORT.
There was a problem hiding this comment.
good point, removed the option to change the port
kpankonen
force-pushed
the
dockerfile
branch
3 times, most recently
from
e3d2cbe to
0419628
Compare
|
@jkodumal is there anything i can do to help get this merged? |
|
@mbarany my fork is pushing to the docker registry if you want to try out the image - https://hub.docker.com/r/kpankonen/ld-relay/ |
|
@kpankonen thanks! I was trying it out yesterday, but ran into the issue of not having the |
|
LGTM now |
|
@kpankonen We'll give it a spin-- I'd guess we'll be able to get it merged in the next few days. |
|
This is great; thanks for the contribution! |
|
Thanks for the review @pkaeding-- I'll resolve the conflicts and merge this. @kpankonen thanks for the contribution! |
|
@kpankonen Merging this in via #20 |
support segments and v3 client
(v6 - #13) simplify context management, don't use a separate type for client-side context info
based on #8, but allows configuration through environment variables