fmjstudios / helm

๐Ÿช– A collection of MIT-licensed Helm Charts
MIT License
8 stars 10 forks source link

fix(charts/paperless-ngx): tika and gotenberg endpoints #20

Closed fty4 closed 1 week ago

fty4 commented 2 months ago

Hello @FMJdev thank you for this repo / chart - I have some issue with the tika and gothenberg endpoints.

What this PR does / why we need it

The tika and gotenberg templated ENV variables are empty by default because it uses a empty values configuration:

https://github.com/fmjstudios/helm/blob/fea914b264718488c3104292280c9d705bf1c3a1/charts/paperless-ngx/templates/configmap.yaml#L190-L200

https://github.com/fmjstudios/helm/blob/fea914b264718488c3104292280c9d705bf1c3a1/charts/paperless-ngx/values.yaml#L329-L344

The new environment variables for the endpoints (k8s service) will be generated as e.g. follows:

<   PAPERLESS_TIKA_ENDPOINT: ""
<   PAPERLESS_TIKA_GOTENBERG_ENDPOINT: ""
---
>   PAPERLESS_TIKA_ENDPOINT: "http://paperless-ngx-tika:9998"
>   PAPERLESS_TIKA_GOTENBERG_ENDPOINT: "http://paperless-ngx-gotenberg:80"

Which issue this PR fixes

no issue opened

Special notes for your reviewer

This implementation adds two new functions to the helpers for each endpoint. If desired they can be merged into one - I added two because it was more likely to fit in the other helper functions.

Also currently the uri can not be overwritten by the values file - only the endpoint which will always add http:// and the port (9998/80) to the environment variable.

Checklist

github-actions[bot] commented 1 month ago

This PR 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.

fty4 commented 3 weeks ago

Please do not close PR - still valid.

FMJdev commented 2 weeks ago

Hey @fty4, first of all sorry for leaving this open for so long. It's been hectic but I'm back now. I will be getting to your PR tomorrow afternoon!

FMJdev commented 1 week ago

Excuse the wait.. As expected stuff took longer again. What a basic oversight on my part ๐Ÿ˜„ I actually prefer two separate functions. I'll catch this stuff with better testing soon. Thank you for the fix @fty4 ! Merging ๐Ÿ‘๐Ÿผ