Skip to content

Commit

Permalink
fix: LAUNCH_DOCKER was removed from compose and config.py; added it back
Browse files Browse the repository at this point in the history
  • Loading branch information
K-Mistele committed Feb 4, 2025
1 parent f88e27a commit ef4ad97
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 1 deletion.
2 changes: 2 additions & 0 deletions docker-compose.development.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ services:
RMQ_USER: ${RMQ_USER:?error}
RMQ_PASSWORD: ${RMQ_PASSWORD:?error}
OPENAI_API_KEY: ${OPENAI_API_KEY:?error}
LAUNCH_DOCKER: true

volumes:
- ./node/config.py:/app/node/config/config.py
- ./node/storage/db/alembic/versions:/app/node/storage/db/alembic/versions
Expand Down
1 change: 1 addition & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ services:
RMQ_USER: ${RMQ_USER:?error}
RMQ_PASSWORD: ${RMQ_PASSWORD:?error}
OPENAI_API_KEY: ${OPENAI_API_KEY:?error}
LAUNCH_DOCKER: true
volumes:
- ./node/config.py:/app/node/config/config.py
- ./node/storage/db/alembic/versions:/app/node/storage/db/alembic/versions
Expand Down
3 changes: 2 additions & 1 deletion node/config.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
from pathlib import Path
import os

# Node
# True if you want to launch node in docker containers, False if you want to run in systemd services
LAUNCH_DOCKER = False
LAUNCH_DOCKER = (os.environ.get('LAUNCH_DOCKER', 'false') == 'true')
NUM_GPUS=0
VRAM=0
DOCKER_JOBS=False
Expand Down

2 comments on commit ef4ad97

@richardblythman
Copy link
Contributor

Choose a reason for hiding this comment

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

@K-Mistele is there a reason we need LAUNCH_DOCKER in the env file? we moved it to get the new compose script working.
its a non sensitive variable.
also LAUNCH_DOCKER will always be true for the docker-compose files so cant we just assume it?

@K-Mistele
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So, the reason I added it as an env variable is that this allows us to simply set it as an environment variable for the node-app service in the compose configuration (whether we're launching the compose files directly, or through the launch script you're building) without ever needing to manually edit it in the node/config.py file.

This is useful because it means the node is aware of when it's being launched in docker without requiring that the user configure anything in config.py. In short, it removes a configuration step - the user doesn't have to manually edit a variable in config.py. Compose configurations can manually set the variable, and otherwise it will be unset (e.g. launching with systemd) resulting in the variable defaulting to False.

Please sign in to comment.