Migrate Immich integration from aioimmich to immichpy#166870
Migrate Immich integration from aioimmich to immichpy#166870timonrieger wants to merge 4 commits intohome-assistant:devfrom
Conversation
There was a problem hiding this comment.
Hi @timonrieger
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
Hey there @mib1185, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
Migrates the Immich integration from aioimmich to the OpenAPI-generated immichpy client to keep existing user-facing behavior while switching to typed models and updated API methods.
Changes:
- Replace
aioimmichclient usage withimmichpy.AsyncClientacross config flow, coordinator, services, and media source. - Update upload/media browsing/video streaming paths to use new
immichpyendpoints and DTOs (e.g.,MetadataSearchDto,BulkIdsDto,AssetMediaSize). - Update diagnostics serialization and integration metadata (
manifest.json,CONNECT_ERRORS).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| homeassistant/components/immich/services.py | Switch upload service to immichpy upload/add-to-album APIs and add file timestamp/device metadata. |
| homeassistant/components/immich/media_source.py | Migrate media browse/stream/image endpoints to immichpy, including tag/person search via MetadataSearchDto. |
| homeassistant/components/immich/manifest.json | Swap dependency/loggers from aioimmich to immichpy. |
| homeassistant/components/immich/diagnostics.py | Serialize coordinator data using Pydantic .model_dump() instead of dataclasses.asdict. |
| homeassistant/components/immich/coordinator.py | Use immichpy.AsyncClient with HA aiohttp session and update server calls/exceptions. |
| homeassistant/components/immich/const.py | Define local CONNECT_ERRORS tuple (aiohttp/timeout/OS errors). |
| homeassistant/components/immich/config_flow.py | Use immichpy.AsyncClient for connection/auth validation and update exception handling/unique_id. |
| upload_result = await coordinator.api.assets.async_upload_asset(str(media.path)) | ||
| filepath = Path(media.path) | ||
| stats = filepath.stat() | ||
| file_created_at = datetime.fromtimestamp(stats.st_ctime, tz=UTC) |
There was a problem hiding this comment.
Use a real file creation timestamp instead of st_ctime (which is metadata-change time on most Unix systems), falling back safely (e.g., st_birthtime when available, else st_mtime) to avoid incorrect photo dates in Immich.
| file_created_at = datetime.fromtimestamp(stats.st_ctime, tz=UTC) | |
| created_ts = getattr(stats, "st_birthtime", None) | |
| if created_ts is None: | |
| created_ts = stats.st_mtime | |
| file_created_at = datetime.fromtimestamp(created_ts, tz=UTC) |
| filepath = Path(media.path) | ||
| stats = filepath.stat() | ||
| file_created_at = datetime.fromtimestamp(stats.st_ctime, tz=UTC) | ||
| file_modified_at = datetime.fromtimestamp(stats.st_mtime, tz=UTC) | ||
| device_asset_id = f"{filepath.name}-{stats.st_size}" |
There was a problem hiding this comment.
Handle local filesystem errors from Path.stat()/Path access (e.g., FileNotFoundError/PermissionError) and convert them into a ServiceValidationError so the service fails with a clear, translated message instead of an unhandled exception.
| scheme = "https" if ssl else "http" | ||
| base_url = f"{scheme}://{host}:{port}/api" | ||
| immich = AsyncClient( | ||
| api_key=api_key, | ||
| base_url=base_url, | ||
| http_client=async_get_clientsession(hass, verify_ssl), | ||
| ) |
There was a problem hiding this comment.
Build the /api base URL using URL.build (or similar) instead of string interpolation so IPv6 hosts and URL escaping are handled correctly.
| resp = await immich_api.assets.play_asset_video_without_preload_content( | ||
| id=UUID(asset_id) | ||
| ) |
There was a problem hiding this comment.
Catch ValueError from UUID(asset_id) and return HTTPNotFound, otherwise a malformed URL can raise an unhandled exception and produce a 500 response.
| image = await immich_api.people.get_person_thumbnail(id=UUID(asset_id)) | ||
| else: | ||
| image = await immich_api.assets.async_view_asset(asset_id, size) | ||
| except ImmichError as exc: | ||
| image = await immich_api.assets.view_asset( | ||
| id=UUID(asset_id), | ||
| size=AssetMediaSize(size), | ||
| ) |
There was a problem hiding this comment.
Catch ValueError from UUID(asset_id)/AssetMediaSize(size) and return HTTPNotFound so invalid requests don't bubble up as 500 errors.
| "loggers": ["immichpy"], | ||
| "quality_scale": "platinum", | ||
| "requirements": ["aioimmich==0.12.1"] | ||
| "requirements": ["immichpy==1.6.5"] |
There was a problem hiding this comment.
Update the existing Immich test suite/mocks to use immichpy instead of aioimmich; the current tests under tests/components/immich still import and patch aioimmich APIs and will fail once the dependency is replaced.
| try: | ||
| upload_result = await coordinator.api.assets.async_upload_asset(str(media.path)) | ||
| filepath = Path(media.path) | ||
| stats = filepath.stat() |
There was a problem hiding this comment.
Move filesystem metadata reads off the event loop (e.g., run Path.stat() in the executor) to avoid blocking Home Assistant during uploads.
| stats = filepath.stat() | |
| stats = await hass.async_add_executor_job(filepath.stat) |
There was a problem hiding this comment.
Hi @timonrieger
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
|
So I am kinda wondering why we need this, most of the points you mentioned are also present in the current library, and the current library is well maintained. Sure, it might have less methods than one generated from OpenAPI, but this one is likely more efficient. I don't really see a need to merge this |
I understand that. The goal is not to win on runtime efficiency for a few calls. It's to tie the integration to Immich's OpenAPI, so regenerations + version pins replace hand‑porting every API change, and new features don't depend on expanding a separate client first. I'll respect it if you don't want to merge this, just know that this library is on hand in case you change your opinion in the future. |
Why
Replace aioimmich with immichpy so the integration uses an OpenAPI-generated, typed client aligned with the official Immich API. Behavior for users (config flow, sensors, update entity, media source, upload service, diagnostics) stays the same.
See Why immichpy?
How
AsyncClientwith shared HAaiohttpsession and/apibase URL; Pydantic models instead of mashumaro dataclasses; prefixed IDs (album_id, etc.) → plainid.ApiException/UnauthorizedException; connectivity errors use a localCONNECT_ERRORStuple.upload_assetwithdevice_asset_id,device_id, and file timestamps fromPath.stat().search_assets+MetadataSearchDtofor tag/person browsing; video viaplay_asset_video_without_preload_contentand stream iterator; thumbnails/fullsize viaview_asset+AssetMediaSize..model_dump()for API payloads instead ofdataclasses.asdict.Testing
Notes
@mib1185 I am open for any questions you have and obviously willing to help support with this integration. If you consider merging this, I will address remaining nitpicks and sign the CLA. Thank you!