Closed cartfisk closed 5 months ago
I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/transmission/4.0.4-r0-pkg-f0b7a686-dev-5e3b7add23cd976ec6e6cf1d8eb49b13b06fdd27-pr-266/index.html https://ci-tests.linuxserver.io/lspipepr/transmission/4.0.4-r0-pkg-f0b7a686-dev-5e3b7add23cd976ec6e6cf1d8eb49b13b06fdd27-pr-266/shellcheck-result.xml | Tag | Passed |
---|---|---|
amd64-4.0.4-r0-pkg-f0b7a686-dev-5e3b7add23cd976ec6e6cf1d8eb49b13b06fdd27-pr-266 | ✅ | |
arm64v8-4.0.4-r0-pkg-f0b7a686-dev-5e3b7add23cd976ec6e6cf1d8eb49b13b06fdd27-pr-266 | ✅ |
I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/transmission/4.0.5-r0-pkg-331410ce-dev-72f04662cff22cea0df0d61e36da3683500c4191-pr-266/index.html https://ci-tests.linuxserver.io/lspipepr/transmission/4.0.5-r0-pkg-331410ce-dev-72f04662cff22cea0df0d61e36da3683500c4191-pr-266/shellcheck-result.xml | Tag | Passed |
---|---|---|
amd64-4.0.5-r0-pkg-331410ce-dev-72f04662cff22cea0df0d61e36da3683500c4191-pr-266 | ✅ | |
arm64v8-4.0.5-r0-pkg-331410ce-dev-72f04662cff22cea0df0d61e36da3683500c4191-pr-266 | ✅ |
This pull request has been automatically marked as stale because it has not had recent activity. This might be due to missing feedback from OP. It will be closed if no further activity occurs. Thank you for your contributions.
@nemchik Made the requested revisions.
I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/transmission/4.0.5-r0-pkg-044b2b9e-dev-b943513a252337cc91929a5e45fd750b76d85e74-pr-266/index.html https://ci-tests.linuxserver.io/lspipepr/transmission/4.0.5-r0-pkg-044b2b9e-dev-b943513a252337cc91929a5e45fd750b76d85e74-pr-266/shellcheck-result.xml | Tag | Passed |
---|---|---|
amd64-4.0.5-r0-pkg-044b2b9e-dev-b943513a252337cc91929a5e45fd750b76d85e74-pr-266 | ✅ | |
arm64v8-4.0.5-r0-pkg-044b2b9e-dev-b943513a252337cc91929a5e45fd750b76d85e74-pr-266 | ✅ |
Thanks for the PR, but I don't see how the paths are hardcoded. There are defaults set in the setting.json
, but that's user configurable. You can change them to whatever paths you like.
I understand being able to set those in env vars would help with getting it up and running with a single command with migration, but we also need to consider that these advertised new vars will lead to confusion and increased barrier of entry for new users, and increased support requests for us.
I don't see how the paths are hardcoded
The lines above create directories when the container starts if they don't already exist. No matter what is configured in transmission's settings.json
, these hardcoded directories will always be re-created on restart even if they are unused and manually removed from the filesystem.
I understand being able to set those in env vars would help with getting it up and running with a single command with migration
I mentioned the migration only to illustrate why it's untenable for me to use the default directories. It's not really an ease-of-setup issue, I would just like to prevent these directories from being re-created every restart because the additional directories are a nuisance when navigating via command line. If you can think of another way to accomplish this behavior without the additional variables I would be happy to rework this PR.
I'd argue against the fact that the addition of two optional and pre-configured variables creates any significant barrier-to-entry but then again I am not familiar with the day-to-day of supporting software like this.
cc: @aptalca
The folder creation is forced, but the path setting is not. You can safely ignore the 2 folders created under /config if you wish to use other paths.
If you want to add logic to not (re)create those 2 folders based on different paths being set in settings.json (behind the scenes, without any additional env vars), I'd be open to that.
If you want to add logic to not (re)create those 2 folders based on different paths being set in settings.json (behind the scenes, without any additional env vars), I'd be open to that.
That sounds fine, I'll give it a shot. Thanks!
I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/transmission/4.0.5-r0-pkg-b53f7895-dev-5d391f8b7fd5d2bc6290c2e9c6a0110623e558bc-pr-266/index.html https://ci-tests.linuxserver.io/lspipepr/transmission/4.0.5-r0-pkg-b53f7895-dev-5d391f8b7fd5d2bc6290c2e9c6a0110623e558bc-pr-266/shellcheck-result.xml | Tag | Passed |
---|---|---|
amd64-4.0.5-r0-pkg-b53f7895-dev-5d391f8b7fd5d2bc6290c2e9c6a0110623e558bc-pr-266 | ✅ | |
arm64v8-4.0.5-r0-pkg-b53f7895-dev-5d391f8b7fd5d2bc6290c2e9c6a0110623e558bc-pr-266 | ✅ |
Here's a new PR w/ revised implementation: https://github.com/linuxserver/docker-transmission/pull/277
Description:
Currently the
complete
andincomplete
download paths are hardcoded. When migrating a large transmission install to this container it is quite difficult to rewrite paths in Transmission's.resume
files. This PR allows users to supply a custom path fordownload-dir
andincomplete-dir
to be used in the init script (creating/owning dirs) and writes the path tosettings.json
.Benefits of this PR and context:
I recently migrated a large (~2000 xfers) transmission environment from Ubuntu to this container. My existing download-dir is named
completed
and I would like to continue using that name without the complete dir interfering with my tab complete. The process of rewriting transmission's .resume files to match the complete naming convention is tedious and prone to errors.How Has This Been Tested?
Ran jenkins-builder!
Source / References:
https://github.com/linuxserver/docker-transmission/issues/265 https://github.com/linuxserver/docker-transmission/issues/256