search

linuxserver / reverse-proxy-confs

These confs are pulled into our SWAG image: https://github.com/linuxserver/docker-swag
GNU General Public License v3.0
1.38k stars 310 forks source link

Update immich.subdomain.conf.sample #713

Open NLZ opened 3 weeks ago

NLZ commented 3 weeks ago

linuxserver.io

Description

Renaming the default container name from immich to immich-server as that's what it called in the official compose file: https://github.com/immich-app/immich/blob/v1.119.1/docker/docker-compose.yml#L12

Benefits of this PR and context

It will work out of the box with the default configs, no need to edit the config or rename the container.

How Has This Been Tested?

Using the default immich-compose.yaml. With the old config it fails and nginx error.log says:

2024/11/02 19:58:48 [error] 1776#1776: *17 immich could not be resolved (3: Host not found), client: xxx.xxx.xxx.xxx, server: immich.*, request: "GET / HTTP/2.0", host: "immich.xxxx.xxx"

After changing it to immich-server it will start working, access.log saying:

84.1.208.124 - - [02/Nov/2024:20:19:03 +0100] "GET /auth/login HTTP/2.0" 200 6298 "-" "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:132.0) Gecko/20100101 Firefox/132.0"
84.1.208.124 - - [02/Nov/2024:20:19:03 +0100] "GET /_app/immutable/chunks/preload-helper.C1FmrZbK.js HTTP/2.0" 200 568 "https://immich.xxxx.xxx/auth/login" "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:132.0) Gecko/20100101 Firefox/132.0"
...

Source / References

Not sure when was any of their container called simply immich by default, maybe never. Looking back the changelog all I can find is removing immich-proxy in v1.88.0 https://github.com/immich-app/immich/releases/tag/v1.88.0

aptalca commented 3 weeks ago

Please see the lines 2 and 3. The config is correct

NLZ commented 3 weeks ago

Right, the comments also needed to be updated, thanks for the reminder

aptalca commented 3 weeks ago

I was saying to leave everything as is. Because the way the readme is right now, it is correct.

NLZ commented 3 weeks ago

So instead of changing the config so it works without any additional change from the users, it's better if everyone who wants to use immich have to rename their container or change the config?

aptalca commented 3 weeks ago

I don't recall the reasoning why it was done like this. Perhaps the upstream container used to be named immich at some point, or perhaps there was no official container and whoever submitted the config used the app name. Or maybe upstream had separate server and frontend containers. Whatever the reason was, our current conf works as intended and lots of people are using it, many with the auto proxy mod. Changing it now would be a breaking change for them for little to no benefit added.

New users follow the instructions at the top of the conf to name the container that provides the gui as directed and it works. I did that myself not long ago 😉

Thanks for the PR, but I personally don't see a high enough benefit to cost ratio to warrant such changes.

NLZ commented 3 weeks ago

The official docker image was always called immich-server (or more specifically immich_server) since 2022. I submitted the immich conf.sample almost a year go, but I made a mistake and the comment instruction said to rename it to immich, so later the config was updated to look for immich but I didn't notice that until now.

Is there any metrics on how many people are actually using the auto proxy mod with immich?

aptalca commented 3 weeks ago

When it comes to docker usage, metrics are quite lacking.

Here's one thing we can do though. Since the current conf uses immich for everything including the conf file name, server name and container name, we could create a second conf that uses immich-server for all of those things. That way we don't break anything for existing users as they'll keep using the old conf and new users can use the new conf with the upstream default container name.