Feat: Add immich server version log#673
Conversation
📝 WalkthroughWalkthroughThe application adds startup-time Immich server version logging for all configured accounts, including per-account failures and warnings for major versions below 3. HttpClient request logs are suppressed unless debug logging is enabled. ChangesServer version logging
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ApplicationStartup
participant ImmichServerVersionLogger
participant IServerSettings
participant IHttpClientFactory
participant ImmichServer
participant ILogger
ApplicationStartup->>ImmichServerVersionLogger: LogServerVersions
ImmichServerVersionLogger->>IServerSettings: Load configured accounts
ImmichServerVersionLogger->>IHttpClientFactory: Create API-authenticated client
IHttpClientFactory->>ImmichServer: GetServerVersionAsync
ImmichServer-->>ImmichServerVersionLogger: Return version
ImmichServerVersionLogger->>ILogger: Log version or warning
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ImmichFrame.WebApi/Helpers/ImmichServerVersionLogger.cs (1)
26-51: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider parallelizing per-account version checks to reduce startup time.
Accounts are processed sequentially with a 5-second timeout each. With multiple unreachable servers, startup blocks for
N × 5seconds. SinceILoggeris thread-safe andIHttpClientFactory.CreateClientsupports concurrent calls, wrapping the loop body inTask.WhenAllwould cap the total wait at 5 seconds regardless of account count.♻️ Proposed refactor: parallel execution via Task.WhenAll
var httpClientFactory = services.GetRequiredService<IHttpClientFactory>(); - foreach (var account in accounts) - { - try - { - var httpClient = httpClientFactory.CreateClient("ImmichApiAccountClient"); - httpClient.UseApiKey(account.ApiKey); - var immichApi = new ImmichApi(account.ImmichServerUrl, httpClient); - - using var cts = new CancellationTokenSource(RequestTimeout); - var version = await immichApi.GetServerVersionAsync(cts.Token); - - logger.LogInformation("Immich server {Url} is running v{Major}.{Minor}.{Patch}", - account.ImmichServerUrl, version.Major, version.Minor, version.Patch); - - if (version.Major < 3) - { - logger.LogWarning("Immich server {Url} is running v{Major}.{Minor}.{Patch}, but this version of ImmichFrame requires Immich v3 or newer. Please update your Immich server.", - account.ImmichServerUrl, version.Major, version.Minor, version.Patch); - } - } - catch (Exception ex) - { - logger.LogWarning("Could not determine Immich server version for {Url}: {Message}", - account.ImmichServerUrl, ex.Message); - } - } + var tasks = accounts.Select(async account => + { + try + { + var httpClient = httpClientFactory.CreateClient("ImmichApiAccountClient"); + httpClient.UseApiKey(account.ApiKey); + var immichApi = new ImmichApi(account.ImmichServerUrl, httpClient); + + using var cts = new CancellationTokenSource(RequestTimeout); + var version = await immichApi.GetServerVersionAsync(cts.Token); + + logger.LogInformation("Immich server {Url} is running v{Major}.{Minor}.{Patch}", + account.ImmichServerUrl, version.Major, version.Minor, version.Patch); + + if (version.Major < 3) + { + logger.LogWarning("Immich server {Url} is running v{Major}.{Minor}.{Patch}, but this version of ImmichFrame requires Immich v3 or newer. Please update your Immich server.", + account.ImmichServerUrl, version.Major, version.Minor, version.Patch); + } + } + catch (Exception ex) + { + logger.LogWarning("Could not determine Immich server version for {Url}: {Message}", + account.ImmichServerUrl, ex.Message); + } + }); + await Task.WhenAll(tasks);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ImmichFrame.WebApi/Helpers/ImmichServerVersionLogger.cs` around lines 26 - 51, Update the per-account processing in the version-logging method to run concurrently with Task.WhenAll instead of awaiting each account sequentially. Move the existing try/catch version-check logic into an async task for each account, preserving the five-second RequestTimeout, logging, and exception handling, then await all account tasks together.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@ImmichFrame.WebApi/Helpers/ImmichServerVersionLogger.cs`:
- Around line 26-51: Update the per-account processing in the version-logging
method to run concurrently with Task.WhenAll instead of awaiting each account
sequentially. Move the existing try/catch version-check logic into an async task
for each account, preserving the five-second RequestTimeout, logging, and
exception handling, then await all account tasks together.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5ab25792-fb4b-4ec3-97e8-71623e479021
📒 Files selected for processing (2)
ImmichFrame.WebApi/Helpers/ImmichServerVersionLogger.csImmichFrame.WebApi/Program.cs
This also removes log noise on level info
Summary by CodeRabbit
New Features
Improvements