search

apogiatzis / docker-compose-livereloader

A docker compose pattern to enable automatic container reloading.
77 stars 12 forks source link

Feedback #1

Open jakajancar opened 4 years ago

jakajancar commented 4 years ago

Bundling a few things into one issue...

  1. First, super clever idea! Very clean, no modification of reloadee, no stuff on host ...

  2. The following in the README had me scratching my head for a moment::

If no RELOAD_DIR variable was set, livereloader automatically watches for changes in that directory.

could clarify as:

If no RELOAD_DIR variable was set, livereloader automatically watches for changes in the mounted directory.

  1. It would be nicer if I didn't have to set container_name (and therefore limit instances to #1), instead reloader could be configured with service name/container prefix and restart all matching.

  2. The reloader finds itself by hardcoded name, could it use the ID instead? How are you expected to have multiple for multiple services?

  3. Perhaps even the name of the restartee could be inferred from the reloader by convention, e.g.:

version: '3'

services:
  test-service:
    ... no requirements ...

  test-service-reloader:
    image: apogiatzis/livereloading
    privileged: true
    volumes:
      - "/var/run/docker.sock:/var/run/docker.sock"
      - ".:/code"

So:

     1. By ID (hostname?) find that I am "test-service-reloader"
     2. Assume reloaded service is called "test-service"
     3. Find all containers called `$COMPOSE_PROJECT_NAME_test-service_*` and reload

  1. I'm not sure I understand the purpose of RELOAD_DIR. If you want to watch only a subdir, then just mount a subdir?

  2. I only use compose in dev. The doc says multiple times to inherit the compose file. I wasn't sure if this is really needed for some reason or not. Turns out not, so maybe better to say it's optional.

  3. Takes a while to exit without stop_signal: SIGKILL defined, would be nice to listen to SIGTERM.

  4. If service crashes, next time you save you get the error "Container to reload not found. Have you set RELOAD_CONTAINER variable? (Or is it runnng?)"