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

feat: single compose file including hwaccel #14461

Closed
wants to merge 1 commit into from

Conversation

mertalev
Copy link
Contributor

@mertalev mertalev commented Dec 3, 2024

Description

The hwaccel.*.yml files are separate for two reasons: to make it easier to set all the appropriate settings without missing anything or caring about the details, and to avoid cluttering the services with a bunch of commented lines.

However, the fact that they're separate files makes them inconvenient, and many platforms don't expose the ability to have multiple Compose files. Uncommenting the extends section and changing the right fields from cpu is simple but also somewhat awkward, and it becomes more awkward in a dev workflow as you have to stash and unstash changes.

This PR moves all of this into the same Compose file and makes it possible to set the transcoding and machine learning backends just through the IMMICH_TRANSCODING_BACKEND and IMMICH_MACHINE_LEARNING_BACKEND environmental variables. This makes it easier to configure with fewer steps, leaves less room for error, and makes it much more convenient for development.

The way it does this is crafty: it defines services for these configs, disables them by assigning a profile to them so they don't actually run, extends the server and machine learning services from the same Compose file, uses environmental variable interpolation in this extends section to allow the extended service to be changed dynamically, and lastly unsets the profiles field in these services so they do in fact run.

How Has This Been Tested?

I tested both with and without these envs and confirmed all immich services run, that these special services don't run, and that the envs affect building and configuration.

@mmomjian
Copy link
Contributor

mmomjian commented Dec 3, 2024

Wow, this might be as cursed as my anchors yaml experiment ;)

@mertalev
Copy link
Contributor Author

mertalev commented Dec 3, 2024

lol it's arguably more cursed - I did it this way because anchors weren't powerful enough

@mertalev mertalev force-pushed the chore/set-image-in-hwaccel branch 2 times, most recently from bc599af to ae0be12 Compare December 3, 2024 05:03
@mertalev mertalev force-pushed the chore/set-image-in-hwaccel branch from ae0be12 to 1134133 Compare December 3, 2024 05:06
@mertalev
Copy link
Contributor Author

On further discussion with the team, the conclusion is that the convenience of this change is outweighed by the complexity and "magic" it adds to the Compose file.

A better alternative is to develop a script that outputs a more tailored file. This can possibly have a UI in the docs so users can configure the Compose and .env files before downloading them. This obviates the need for the hwaccel files and modifying the Compose file, and can make other features like mounting external libraries less error-prone. It would also accomplish this with a clean and minimal Compose file without any unnecessary clutter.

@mertalev mertalev closed this Dec 10, 2024
@jrasm91 jrasm91 deleted the chore/set-image-in-hwaccel branch December 10, 2024 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants