add env for logfile and config file

ksurl commented 1 year ago

[x] I have read the contributing guideline and understand that I have made the correct modifications

Description:

add 2 environment variables for flexget parameters

Benefits of this PR and context:

support custom paths for log and config files

How Has This Been Tested?

tested locally with volume override of the run file for flexget s6 service

LinuxServer-CI commented 1 year ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/flexget/v3.7.10-pkg-3aa94932-dev-f6ae927aaa33f0704abde07f715b1be9b910d501-pr-3/index.html https://ci-tests.linuxserver.io/lspipepr/flexget/v3.7.10-pkg-3aa94932-dev-f6ae927aaa33f0704abde07f715b1be9b910d501-pr-3/shellcheck-result.xml	Tag	Passed
amd64-v3.7.10-pkg-3aa94932-dev-f6ae927aaa33f0704abde07f715b1be9b910d501-pr-3	✅
arm64v8-v3.7.10-pkg-3aa94932-dev-f6ae927aaa33f0704abde07f715b1be9b910d501-pr-3	✅

ksurl commented 1 year ago

Thoughts on supporting custom path for config and log? I was expecting config to be looked for at /config and I usually put logs in a logs subfolder since it rotates the log and starts to clutter the main folder.

thespad commented 1 year ago

I'm fine with the log file change, but the reason I put the config file where it is, is because:

By default FlexGet tries to find config.yml from several places, in this order:

The current path
The virtualenv path (only if you are running FlexGet installed in a virtualenv)
~/.config/flexget (only on Linux)

Which means if you want to run CLI commands and the config yml isn't in one of those locations, you have to manually pass the --config arg with the full path every time.

If we let people move it around the most likely outcome is we generate a bunch of support issues asking why the CLI isn't working properly.

ksurl commented 1 year ago

I think that it only matters if you are not running the daemon. Since the service start includes -c specifying the location. I exec'd into my running container and it defaulted to / since workdir is not specified, and flexget check came back ok. Also you can still leave it default to .flexget in the run file so it should find it if not specified on the fly. My workaround is a symlink for .flexget->/config.

thespad commented 1 year ago

OK, if we're going to merge this then the init needs updating as well.

Multiple sections here: https://github.com/linuxserver/docker-flexget/blob/main/root/etc/s6-overlay/s6-rc.d/init-flexget-config/run#L4-L22 assume the default config location and will need updating.

ksurl commented 1 year ago

ok. I think it's fine to leave all the defaults at .flexget, but have the config lock file check use the variable so it will catch any non default location.

LinuxServer-CI commented 1 year ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/flexget/v3.7.10-pkg-5efc10df-dev-3f00aef28281386679720bc75b4e340278854203-pr-3/index.html https://ci-tests.linuxserver.io/lspipepr/flexget/v3.7.10-pkg-5efc10df-dev-3f00aef28281386679720bc75b4e340278854203-pr-3/shellcheck-result.xml	Tag	Passed
amd64-v3.7.10-pkg-5efc10df-dev-3f00aef28281386679720bc75b4e340278854203-pr-3	✅
arm64v8-v3.7.10-pkg-5efc10df-dev-3f00aef28281386679720bc75b4e340278854203-pr-3	✅

LinuxServer-CI commented 1 year ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/flexget/v3.7.11-pkg-60605567-dev-d84be1960a1e7d6a0e432558bfa6f98755a62b78-pr-3/index.html https://ci-tests.linuxserver.io/lspipepr/flexget/v3.7.11-pkg-60605567-dev-d84be1960a1e7d6a0e432558bfa6f98755a62b78-pr-3/shellcheck-result.xml	Tag	Passed
amd64-v3.7.11-pkg-60605567-dev-d84be1960a1e7d6a0e432558bfa6f98755a62b78-pr-3	✅
arm64v8-v3.7.11-pkg-60605567-dev-d84be1960a1e7d6a0e432558bfa6f98755a62b78-pr-3	✅

linuxserver / docker-flexget