Skip to content

feat(server): lighter buckets #17831

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

Merged
merged 93 commits into from
May 19, 2025
Merged

feat(server): lighter buckets #17831

merged 93 commits into from
May 19, 2025

Conversation

midzelis
Copy link
Collaborator

Description

This is the server side changes needed for #17719

Note: this PR is draft until #17719 is merged, since after #17719 is merged, the web code will need to be adjusted to consume the new response format instead of the old response format. #17719 is converted the old format to a new format internally, so after this change goes in, it would be able to directly load the result.

This reduces the size of the getTimeBucket response by approximately 88-92%

@midzelis midzelis changed the base branch from main to lighter_buckets_web April 29, 2025 01:55
@midzelis midzelis force-pushed the lighter_buckets_server branch from c1b2366 to 077703a Compare April 29, 2025 02:00
Copy link
Member

@danieldietzler danieldietzler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM! Great work on this you two, can't wait for it to land 🚀

@danieldietzler danieldietzler requested a review from jrasm91 May 17, 2025 10:29
Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good. I have no idea why you have all the mobile SDK generator changes which should not be related to this PR at all, since mobile doesn't use the time buckets.

@mertalev
Copy link
Contributor

Looks pretty good. I have no idea why you have all the mobile SDK generator changes which should not be related to this PR at all, since mobile doesn't use the time buckets.

The generator doesn't make list elements nullable, so it ends up giving type errors. I patched the generator so it makes the right types.

Base automatically changed from lighter_buckets_web to main May 18, 2025 02:57
Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM

@alextran1502 alextran1502 merged commit e7edbcd into main May 19, 2025
52 checks passed
@alextran1502 alextran1502 deleted the lighter_buckets_server branch May 19, 2025 21:40
@github-actions github-actions bot removed the preview label May 19, 2025
savely-krasovsky pushed a commit to savely-krasovsky/immich that referenced this pull request Jun 8, 2025
* feat(web): lighter timeline buckets

* GalleryViewer

* weird ssr

* Remove generics from AssetInteraction

* ensure keys on getAssetInfo, alt-text

* empty - trigger ci

* re-add alt-text

* test fix

* update tests

* tests

* missing import

* feat(server): lighter buckets

* fix: flappy e2e test

* lint

* revert settings

* unneeded cast

* fix after merge

* Adapt web client to consume new server response format

* test

* missing import

* lint

* Use nulls, make-sql

* openapi battle

* date->string

* tests

* tests

* lint/tests

* lint

* test

* push aggregation to query

* openapi

* stack as tuple

* openapi

* update references to description

* update alt text tests

* update sql

* update sql

* update timeline tests

* linting, fix expected response

* string tuple

* fix spec

* fix

* silly generator

* rename patch

* minimize sorting

* review

* lint

* lint

* sql

* test

* avoid abbreviations

* review comment - type safety in test

* merge conflicts

* lint

* lint/abbreviations

* remove unncessary code

* review comments

* sql

* re-add package-lock

* use booleans, fix visibility in openapi spec, less cursed controller

* update sql

* no need to use sql template

* array access actually doesn't seem to matter

* remove redundant code

* re-add sql decorator

* unused type

* remove null assertions

* bad merge

* Fix test

* shave

* extra clean shave

* use decorator for content type

* redundant types

* redundant comment

* update comment

* unnecessary res

---------

Co-authored-by: mertalev <[email protected]>
Co-authored-by: Alex <[email protected]>
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.

5 participants