Skip to content

Get env vars from .env file#554

Merged
HassanAbouelela merged 3 commits into
python-discord:masterfrom
ChrisLovering:docker-compse-env-file
Feb 8, 2021
Merged

Get env vars from .env file#554
HassanAbouelela merged 3 commits into
python-discord:masterfrom
ChrisLovering:docker-compse-env-file

Conversation

@ChrisLovering
Copy link
Copy Markdown
Member

@ChrisLovering ChrisLovering commented Jan 13, 2021

Description

Removed explicit mentioned of env vars from docker compose, replaced with an env_file

Reasoning

This means users of the docker-compose workflow won't need to worry about accidentally commiting their docker-compose file which contains all their secrets!

This will pull all of the env vars from the .env file in the project root dir

See this chat for context.

Additional Details

Before merging we should:

  • Update the contributing wiki with these changes
  • Announce this change, so users can extract their secrets from the docker-compose file and put them into a .env file before pulling

Did you:

  • Join the Python Discord Community?
  • If dependencies have been added or updated, run pipenv lock?
  • Lint your code (pipenv run lint)?
  • Set the PR to allow edits from contributors?

@HassanAbouelela HassanAbouelela added area: CI Related to continuous intergration and deployment type: feature Relating to the functionality of the application. status: planning Discussing details and removed area: CI Related to continuous intergration and deployment labels Jan 13, 2021
@ChrisLovering
Copy link
Copy Markdown
Member Author

ChrisLovering commented Jan 13, 2021

Some alternatives to this approach would be:

  • Explicitly list all env vars in the enviroment section of docker-compose.yml so that they get pulled into the container from .env
  • Tell users to put their .env file inside ./bot/ (So it gets copied to the container) and use dotenv in contants.py to load from there

@Xithrius Xithrius added level: 0 - beginner status: needs review Author is waiting for someone to review and approve and removed status: planning Discussing details labels Jan 24, 2021
Comment thread docker-compose.yml
- BOT_ADMIN_ROLE_ID
- CHANNEL_DEVLOG
- CHANNEL_COMMUNITY_BOT_COMMANDS
- REDIS_HOST=redis
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If this doesn't already have a default value set in Python then it should. Alternatively, the default can remain in the compose file but I prefer defining it in Python.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The default is currently set to redis.default.svc.cluster.local

host = environ.get("REDIS_HOST", "redis.default.svc.cluster.local")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Okay, that's the production host and it should stay that way. Therefore, it's best to leave it set to redis within the compose file since it can spin up a redis container. If running without Docker Compose, I presume users are already told they have to manually enable fakeredis?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've added the default back in.
They are, but it's a little buried in the lancebot wiki atm https://pythondiscord.com/pages/contributing/sir-lancebot/sir-lancebot-env-var-reference/
I'm planning on drafting a guide to be added onto the wiki for the docker setup (As it's not currently documented).

Comment thread docker-compose.yml
Copy link
Copy Markdown
Contributor

@HassanAbouelela HassanAbouelela left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for updating this.

@HassanAbouelela HassanAbouelela merged commit 3da4fb8 into python-discord:master Feb 8, 2021
@ChrisLovering ChrisLovering deleted the docker-compse-env-file branch March 6, 2021 14:45
@Xithrius Xithrius removed the status: needs review Author is waiting for someone to review and approve label Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: feature Relating to the functionality of the application.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants