Team-TAU / tau

TAU- Twitch API Unifier, a containerized relay/proxy to unify the WebHook- and WebSocket-based real-time Twitch APIs under a single (local) WebSocket connection.
MIT License
154 stars 39 forks source link

added supervisord-stdout, refactor of Dockerfile #39

Closed rexroof closed 3 years ago

rexroof commented 3 years ago

goal here was to add supervisord-stdout to the container to get all lots on the container stdout.

Also rearranged/updated the Dockerfile along the way and changes to the docker-compose.

0-vortex commented 3 years ago

good quality of life, not judging django internals, redis type local/external is the last thing I would change !

FiniteSingularity commented 3 years ago

Thanks for the great PR @rexroof. This definitely does simplify some things. However, one thing we need to be careful of- in removing the container_name values from docker-compose, we'll likely end up breaking a lot of installs, as the user's .env file will need to be updated with the new database hostname (db instead of tau-db). Off the top of my head, I see a few different ways to address this:

  1. Change the service label in the docker-compose.yml file from db to tau-db.
  2. Somehow notify the user that they need to update their .env file.
  3. Now that the database hostname/url is fixed (to just db if I am interpreting this correctly), get rid of the DJANGO_DB_URL from .env completely, then do something like this in the docker-compose.yml file:
    app:
    restart: unless-stopped
    env_file: .env
    depends_on:
      - db
      - redis
    build:
      context: .
    volumes:
      - ./:/code:cached
    environment:
      -  DJANGO_DB_HOST: ${DJANGO_DB_HOST:-db}
    ports:
      - ${PORT:-8000}:${PORT:-8000}

    In other words- create a new DJANGO_DB_HOST environment variable to replace the old DJANGO_DB_URL, and default it to db. I'd also need to update the django config code to use this variable for the database configuration. That way, if a user wants to use a separate postgres install for some reason, they can override it, and it wont break any existing installations. I'm leaning towards 3, and would appreciate any thoughts on this.

I also need to see if removing loadenv breaks the non-docker-compose use. I think it might (and looking back on my notes, that was the reason I implemented it). I'm sure there is a much better way do what I am doing, but first, lets get the db config piece working without being a breaking change.

FiniteSingularity commented 3 years ago

I just merged a bunch into @rexroof's code, to further simplify the docker files, .env, and initial setup. I'm going to do some additional testing on render.com for the single-container install, and if all works well, will merge the PR.