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

Simplify deploy with docker-compose #1965

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .env.dist
@@ -0,0 +1,2 @@
# Uncomment line below on production server to skip docker-compose -f options
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the .env.dist file automatically loaded? I cannot find any information regarding this on https://docs.docker.com/compose/env-file/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not.

Typically on repo setup, cp .env.dist .env should be run, and .env.dist is basically an example that lives with the repo.

Or you can use --env-file .env.dist as an option to docker-compose commands, although that's less nice from a UX standpoint.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case shouldn't we rename this file .env.prod and uncomment the env var?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say that's probably appropriate considering this project's existing setup. The word dist is a bit incongruent with what this repo currently has.

#COMPOSE_FILE=docker-compose.yml:docker-compose.prod.yml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also add a default SERVER_NAME env var here to hint users that they can use this env var to configure the hostname (cc @francislavoie).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good and we can add more envs like APP_SECRET, MERCURE_PUBLISHER_JWT_KEY, MERCURE_SUBSCRIBER_JWT_KEY but is it a good PR to do that?