linuxserver / docker-jellyfin

GNU General Public License v3.0
647 stars 97 forks source link

Allow configuration override. #113

Closed istvan-andrasi closed 3 years ago

istvan-andrasi commented 3 years ago

linuxserver.io



Description:

This change let's me override the server config with JELLYFIN_*_DIR env vars, so I'm free to relocate specific Jellyfin dirs instead of being forced into a predefined structure.

Also, I expect the env variables mentioned by the Jellyfin documentation to work with this image as well.

Benefits of this PR and context:

JELLYFIN_*_DIR env variables allow the users to relocate the data, config, cache and log directories, if they want to. If those variables are not specified, it works the same way as before.

How Has This Been Tested?

  1. Try to use configuration env vars:

    docker run --name jellyfin_test \
      -v /tmp/jellyfin-data:/data \
      -v /tmp/jellyfin-config:/config \
      -v /tmp/jellyfin-cache:/cache \
      -v /tmp/jellyfin-logs:/logs \
      -e JELLYFIN_DATA_DIR=/data \
      -e JELLYFIN_CONFIG_DIR=/config \
      -e JELLYFIN_CACHE_DIR=/cache \
      -e JELLYFIN_LOG_DIR=/logs \
      ghcr.io/linuxserver/jellyfin
  2. The logs clearly show they are not picked up:

    Main: Environment Variables: ["[JELLYFIN_CONFIG_DIR, /config]", "[JELLYFIN_DATA_DIR, /config/data]", "[JELLYFIN_LOG_DIR, /config/log]", "[JELLYFIN_CACHE_DIR, /config/cache]"]
  3. Run bash in container and make proposed change to startup script.

    docker exec -it jellyfin_test /bin/bash
    # ... edit file and save it ...
  4. Restart container:

    docker stop jellyfin_test && docker start -a jellyfin_test
  5. Log shows the environment variables are picked up:

    Main: Environment Variables: ["[JELLYFIN_CACHE_DIR, /cache]", "[JELLYFIN_LOG_DIR, /logs]", "[JELLYFIN_DATA_DIR, /data]", "[JELLYFIN_CONFIG_DIR, /config]"]

Source / References:

N/A

aptalca commented 3 years ago

This would create a support nightmare with little benefit

LinuxServer-CI commented 3 years ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/jellyfin/10.7.5-1-pkg-3da03dfc-pr-113/index.html https://ci-tests.linuxserver.io/lspipepr/jellyfin/10.7.5-1-pkg-3da03dfc-pr-113/shellcheck-result.xml

istvan-andrasi commented 3 years ago

This would create a support nightmare with little benefit

Please elaborate. It only affects the four environment variables which had no effect until now, and by default they have the same values they used to have.

Thanks.

aptalca commented 3 years ago

The more users deviate from the standardized config, the harder it is for us to help troubleshoot when they have issues.

istvan-andrasi commented 3 years ago

Okay thanks.