Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update immich.subdomain.conf.sample #713

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

NLZ
Copy link
Contributor

@NLZ NLZ commented Nov 2, 2024

linuxserver.io


  • I have read the contributing guideline and understand that I have made the correct modifications

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

Use the default immich-server container name as per the official compose
@aptalca
Copy link
Member

aptalca commented Nov 2, 2024

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

@NLZ
Copy link
Contributor Author

NLZ commented Nov 2, 2024

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

@aptalca
Copy link
Member

aptalca commented Nov 2, 2024

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

@NLZ
Copy link
Contributor Author

NLZ commented Nov 2, 2024

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
Copy link
Member

aptalca commented Nov 2, 2024

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
Copy link
Contributor Author

NLZ commented Nov 5, 2024

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
Copy link
Member

aptalca commented Nov 5, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants