-
Notifications
You must be signed in to change notification settings - Fork 14
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
Make ports exposed in docker compose configurable #2858
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
${EXPOSE_REVIEW_APPROVED}
@@ -108,15 +108,15 @@ services: | |||
environment: | |||
- LOCALSTACK_HOST=localstack | |||
ports: | |||
- "4566:4566" | |||
- "${EXPOSE_LOCALSTACK_PORT:-4566}:4566" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you want to set the environment variables? If you set them in .env
, they will only be used by the Makefile
, and not if VS Code brings the container up. If you set them in your .bashrc
, then all the variable names should contain TECKEN
, so they don't conflict with variable names for other services.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docker compose automatically picks up variables from .env https://docs.docker.com/compose/environment-variables/set-environment-variables/#substitute-with-an-env-file
I made sure it works by exiting vscode, running docker compose -f docker-compose.yml -f .devcontainer/docker-compose.yml down
, opening vscode, waiting for the devcontainer to finish starting, and finally checking the exposed ports via docker compose -f docker-compose.yml -f .devcontainer/docker-compose.yml ps
. the overrides that i had set in .env were used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, nice! I didn't know. We should make use of that for the build arguments. They are currently passed in explicitly from the Makefile
, but we should add them to docker-compose.yml
instead.
343f5ad
to
79a37be
Compare
see also mozilla-services/eliot#113
per https://github.com/mozilla-services/tecken/pull/2857/files#r1430382049
with devcontainer and its docker-compose dependencies running even when vscode isn't open, ports conflict between tecken, eliot, and socorro, so this PR makes the exposed ports configurable via environment variables, which can be set in
.env
in each project to avoid overlap.