search

linuxserver / docker-transmission

GNU General Public License v3.0
582 stars 180 forks source link

Update 20-config to allow default download directory and incomplete directory to be set via environment variables #202

Closed Klubas closed 1 year ago

Klubas commented 2 years ago

linuxserver.io

Description:

Added support for configuring the default download directory and the directory used to store incomplete downloads via the DOWNLOAD_DIR and INCOMPLETE_DIR environment variables.

Benefits of this PR and context:

With this change its possible to set de default download dir just setting the env var on docker-compose.yml, without needing any additional files and a custom Dockerfile.

How Has This Been Tested?

Run the container locally using a docker-compose.yml file to see if the changes where applied correctly to the settings.json file.

Source / References:

LinuxServer-CI commented 2 years ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/transmission/3.00-r5-pkg-e8beb90f-pr-202/index.html https://ci-tests.linuxserver.io/lspipepr/transmission/3.00-r5-pkg-e8beb90f-pr-202/shellcheck-result.xml

LinuxServer-CI commented 2 years ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/transmission/3.00-r5-pkg-e8beb90f-pr-202/index.html https://ci-tests.linuxserver.io/lspipepr/transmission/3.00-r5-pkg-e8beb90f-pr-202/shellcheck-result.xml

hacketiwack commented 2 years ago

This option was existing in the past and was removed.

The bigger problem here is that each time the container is started, the default complete and incomplete dirs are created. Therefore, no matter what you put in these variables, the default directories will be created:

# make folders
mkdir -p \
    /downloads/{complete,incomplete} /watch

I opened this pull request here which is meant to solve your problem as well.

IMHO, you should configure your directories in the settings.json file and deploy the file at the same time as your container. I'm personally really skeptical about adding more and more environment variables for elements that are already present in the settings.json configuration file.

Klubas commented 2 years ago

I didn't though that creating the default dirs despite the env vars being set was a big problem, that's why I left it there, but I agree it's not the ideal scenario. I could change the script to only create the folders that will be really used, but your solution is definetely more elegant.

github-actions[bot] commented 2 years ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

hacketiwack commented 2 years ago

Is there still someone maintaining this repository?

github-actions[bot] commented 2 years ago

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Cronocide commented 1 year ago

It makes the most sense to me to make an ENV var that dictates whether or not the user is specifying their own settings.json file, that way the rest of the configuration needed can be done by the user without opening more PRs for every setting that needs changed.

drizuid commented 1 year ago

I see no benefit to this PR. downloads directory can already be set within the transmission webui and any transmission remote client. Editing settings.json can also accomplish this. Adding more vars would just confuse the majority of our users.

in reply to https://github.com/linuxserver/docker-transmission/pull/202#issuecomment-1259876081 a user can already stop the container and drop in a settings.json to the config path they've selected, there is no need for a variable to accomplish this.

elvismercado commented 1 year ago

I do see value in this. The benefit for setting downloads-dir and incomplete-dir through environment variables would be the ease of configuration. I could setup 1 docker-compose file (or unraid docker template) without having to do extra steps separately in the settings.json file or any remote client. It would start and run as needed.

See the feature added here as an example: https://haugene.github.io/docker-transmission-openvpn/config-options/ "You may still override Transmission options by setting environment variables if that's your thing. The variables are named after the transmission config they target but are prefixed with TRANSMISSION_, capitalized, and - is converted to _."