-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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(server): sidecar in asset_files #16827
base: main
Are you sure you want to change the base?
Conversation
etnoy
commented
Mar 12, 2025
- add new sidecar type in asset_files
- remove sidecarPath from asset
- create dedicated assetFileRepository
…-app/immich into feat/inline-offline-check
…/inline-offline-check
…/inline-offline-check
…/inline-offline-check
…/inline-offline-check
…/inline-offline-check
…/inline-offline-check
…/inline-offline-check
…/inline-offline-check
…/inline-offline-check
…-app/immich into feat/inline-offline-check
…/inline-offline-check
…/inline-offline-check
…/inline-offline-check
…/inline-offline-check
…/sidecar-asset-files
…/sidecar-asset-files
ae1d2ee
to
43eb083
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's not only the database table shift here but also a refactor around how sidecar files are handled/discovered in general, right? I think that'd be easier to review as a separate PR.
@@ -18,12 +18,15 @@ export class AssetFileEntity { | |||
id!: string; | |||
|
|||
@Index('IDX_asset_files_assetId') | |||
@Column() | |||
@Column({ nullable: true, default: null }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a good idea to make this nullable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sidecars are now discovered independently of files in library scans can are possibly orphans
name = 'Test1741731070431'; | ||
|
||
public async up(queryRunner: QueryRunner): Promise<void> { | ||
await queryRunner.query(`ALTER TABLE "assets" DROP COLUMN "sidecarPath"`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this move the existing sidecar paths over to asset_files rather than just dropping them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll think about this later
@GenerateSql({ | ||
params: [{ libraryId: DummyValue.UUID, importPaths: [DummyValue.STRING], exclusionPatterns: [DummyValue.STRING] }], | ||
}) | ||
async detectOfflineExternalSidecars( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sidecars are already treated as malleable, so I don't think sidecar handling should need any special cases for external.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a speedup that we can do for external assets
…/sidecar-asset-files
…/sidecar-asset-files
43eb083
to
ec9d538
Compare