add optional vars for custom complete/incomplete paths #266
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for opening this pull request! Be sure to follow the pull request template!
|
I am a bot, here are the test results for this PR:
|
There was a problem hiding this comment.
The $ should not be used when setting a variable in a shell script (it can be used to supply a variable as part of the value, but not in front of the variable being set).
Correct ex:
MY_VAR="My Val and $OTHER_VAR"Incorrect ex:
$MY_VAR="My Val and $OTHER_VAR"
|
I am a bot, here are the test results for this PR:
|
|
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:
|
|
Thanks for the PR, but I don't see how the paths are hardcoded. There are defaults set in the 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. |
The lines above create directories when the container starts if they don't already exist. No matter what is configured in transmission's
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. |
That sounds fine, I'll give it a shot. Thanks! |
|
I am a bot, here are the test results for this PR:
|
|
Here's a new PR w/ revised implementation: #277 |
Description:
Currently the
completeandincompletedownload paths are hardcoded. When migrating a large transmission install to this container it is quite difficult to rewrite paths in Transmission's.resumefiles. This PR allows users to supply a custom path fordownload-dirandincomplete-dirto 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
completedand 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:
#265
#256