-
Notifications
You must be signed in to change notification settings - Fork 15
feat: Isolate environment configuration from code #1399
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
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.
Please add the env files without the .example to the gitignore
.env.backend.example
Outdated
DASH_DB_PASSWORD=${DASH_DB_PASSWORD:-admin} | ||
DASH_DB_HOST=dashboard_db | ||
DASH_DB_PORT=${DASH_DB_PORT:-5432} | ||
USE_DASHBOARD_DB=${USE_DASHBOARD_DB:-False} No newline at end of file |
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.
nit: leave an empty line at the end of file
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.
Updated/amended, please check
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.
The real environment variables for the database are POSTGRES_USER
, POSTGRES_PASSWORD
and POSTGRES_DB
, even though there were being overriden by those other names. You can use the same syntax as it was being used before (like POSTGRES_USER=${DASH_DB_USER:-admin}
)
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.
Updated/amended, please check
4bdbf5b
to
7844123
Compare
Updated/amended, please check |
Move environment variable definitions from docker-compose.yml to dedicated .env files. This change aligns with the 12factor.net recommendation of storing configuration in the environment. - Introduces .env.backend.example, .env.db.example, and .env.proxy.example to serve as templates. - Modifies docker-compose.yml to use `env_file` for each service. - Ensures that .env files are ignored by git. Signed-off-by: Denys Fedoryshchenko <[email protected]>
7844123
to
2411904
Compare
Oh... I was about to approve this PR and add a not that we need to be careful with this because we need to make sure that staging/production have those .env files, and I just saw that the Github CI already broke. The docker services for the integration tests are not being able to run because it won't find any of the .env.something files, could you check that for us? |
On staging/deployment i will deal with that, i will check workflows now. |
Signed-off-by: Denys Fedoryshchenko <[email protected]>
I think that part is fine now, is other errors relevant? |
The solution was easier than I though 😅 |
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.
LGTM
Move environment variable definitions from docker-compose.yml to dedicated .env files. This change aligns with the 12factor.net recommendation of storing configuration in the environment.
env_file
for each service.