search

truenas / charts

TrueNAS SCALE Apps Catalogs & Charts
BSD 3-Clause "New" or "Revised" License
302 stars 290 forks source link

Unable to override transmission predefined envariable varibles for configuration #1694

Closed funkycode closed 11 months ago

funkycode commented 11 months ago

https://github.com/truenas/charts/blob/206ce2ceefb6103391f3cb94c50aa56edd96ffb1/community/transmission/1.1.1/templates/_configuration.tpl#L10

There is no option to change default predefined settings from tpl above. When adding one of the existing in the definition variables as custom env variables you would get an error:

Error: [EFAULT] Failed to update App: Container - Environment Variable [TRANSMISSION__INCOMPLETE_DIR_ENABLED] in [envList] tried to override the Environment Variable that is already defined in [ConfigMap - transmission-config]

I assume those should be taken out as parameters to values.yaml with defaults, so it would be possible to add them to app edit page. Or need to for range over those and avoid adding if it set via custom envs

stavros-k commented 11 months ago

It's hardcoded because the storage volume it also configured on this location. Why do you want to override it?

funkycode commented 11 months ago

Well, because I used to have own app/chart for transmission and I have already few hundred torrents with path set to different folder in container, I managed by adding additional mount, wanted to set it back to default one in config. Moreover I wanted to disable incomplete download, which seems to be tricky too.

In general it is possible to take this one out and use it both for config and for mount point in helm template.

stavros-k commented 11 months ago

So far if I understand correctly, you want to be able to:

What is the benefit of disabling the incomplete directory? Isn't it better to have it separate so you can for example point that to an faster storage medium? (or just leave it on the same storage medium as downloads) This could probably be added as an option. But how transmission will handle this: User installs app with incomplete enabled. starts a torrent and midway you disable the incomplete storage. (or the other way around)

Regarding the download path, the reason for making it configurable is for existing torrents that had different paths, right? Isn't there a way to change the path on all of them to point to the new directory?

funkycode commented 11 months ago

I can change the path on all my torrents, it would require few lines program or script to use transmission API calls. But i think you are missing the point where the usage and configuration of the application is personal and application itself has those parameters as configurable, yet here it is defined per usage that suites the maintainer of chart. For example the same way the port can be just hardcorded and you could state "well this port works, why not simply use it then", but we do understand that it can benefit to be able to change it, right?

In general i think that all configs saved in configmap limits them to be changed to from within the app as configmaps are readonly, i do understand that default listen address and rpc disabled would make it less user friendly though. Thus I think the better approach is to actually export in configuration those two that would be translated into envs, while the rest leave to user to decide. as per mount i think it would be better to have incomplete mount is optional and both download and incomplete volumes to have additional parameter where it is mounted inside container with default value as it is hardcorded today. Possibly having it also passed as download directory.

stavros-k commented 11 months ago

I can change the path on all my torrents, it would require few lines program or script to use transmission API calls.

I was thinking if there was an built-in way to easily achieve that, I don't wanna have users use/write scripts to do that of course!

But i think you are missing the point where the usage and configuration of the application is personal and application itself has those parameters as configurable, yet here it is defined per usage that suites the maintainer of chart. For example the same way the port can be just hardcorded and you could state "well this port works, why not simply use it then", but we do understand that it can benefit to be able to change it, right?

I'm just looking what are the exact use cases, so I can adapt it best. But also the maintenance part have to be considered. Majority of apps don't "care" where the storage is mounted, and if they do, adjusting the variable, will pick everything up. So exposing everything on every app does not make sense, goal is to make the installer as simple as possible. If you expose too much, then you lose the point of having an "app", as it would be the same as configuring the app via Custom App button.

In general i think that all configs saved in configmap limits them to be changed to from within the app as configmaps are readonly, i do understand that default listen address and rpc disabled would make it less user friendly though. Thus I think the better approach is to actually export in configuration those two that would be translated into envs, while the rest leave to user to decide. as per mount i think it would be better to have incomplete mount is optional and both download and incomplete volumes to have additional parameter where it is mounted inside container with default value as it is hardcorded today. Possibly having it also passed as download directory.

So what I'm thinking is:

I'm not sure that exposing the rpc toggle is a good idea tho. AFAIK this is for the webui and/or api calls. So I'm not sure how someone would use it besides CLI.

Is there anything I missed? or something that you think should be handled differently?

michaelr0 commented 11 months ago

Hi @stavros-k,

Thanks for adding this into https://github.com/truenas/charts/pull/1727

There appears to be a bug/issue with it, where the download dir setting is not being set correctly in Transmission. I think this is due to some referrences to downloadsDir vs downloadDir creating a mismatch