feat: Add additional env variables for Machine Learning #15326
Conversation
github-actions
bot
added
documentation
1-tempest
changed the title
|
📖 Documentation deployed to pr-15326.preview.immich.app |
machine-learning/app/config.py
Outdated
| from pydantic_settings import BaseSettings, SettingsConfigDict | ||
| from rich.console import Console | ||
| from rich.logging import RichHandler | ||
| from uvicorn import Server | ||
| from uvicorn.workers import UvicornWorker | ||
|
|
||
|
|
||
| class ClipSettings(BaseModel): | ||
| model: str | None = None |
There was a problem hiding this comment.
I've been thinking - this interface assumes the same model name will be used for all parts of a task. This is currently true, but might not hold in the future. I think the other approach of doing MACHINE_LEARNING_PRELOAD__CLIP__TEXTUAL=<model name> etc., while more verbose, will be more future-proof so we don't need to make another breaking change down the line.
There was a problem hiding this comment.
That's a fair point. Instead of 3 env variables per model, shall I drop it down to two strings instead of a string and 2 booleans?
| @@ -76,19 +76,32 @@ async def lifespan(_: FastAPI) -> AsyncGenerator[None, None]: | |||
|
|
|||
| async def preload_models(preload: PreloadModelData) -> None: | |||
There was a problem hiding this comment.
Can you add deprecation warnings for the old envs?
There was a problem hiding this comment.
I'm not sure how I'd go about that, just as a comment or something more?
There was a problem hiding this comment.
You can check if the fallback env is not None and call log.warn saying it's deprecated and you should use X instead.
There was a problem hiding this comment.
That makes sense, thanks much!
Add additional variables to only preload partial models.
MACHINE_LEARNING_PRELOAD__CLIPhas been depreciated and replaced withMACHINE_LEARNING_PRELOAD__CLIP__MODELMACHINE_LEARNING_PRELOAD__FACIAL_RECOGNITIONhas been depreciated and replaced withMACHINE_LEARNING_PRELOAD__FACIAL_RECOGNITION__MODEL