Skip to content
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(mobile): render assets on device by default - with stream #10513

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

alextran1502
Copy link
Contributor

  • feat(mobile): render asset on device by default
  • remove unused service
  • feat(mobile): render asset on device by default - with stream

@alextran1502 alextran1502 changed the base branch from main to feat/mobile/render-local-asset-by-default June 20, 2024 21:17
@alextran1502 alextran1502 changed the title feat/mobile/render local asset by default stream feat(mobile): render assets on device by default - with stream Jun 20, 2024
@alextran1502 alextran1502 changed the base branch from feat/mobile/render-local-asset-by-default to main June 20, 2024 22:19
@alextran1502 alextran1502 requested a review from fyfrey June 21, 2024 03:57
await for (final onDevice in _hashService.getHashedAssets(ape)) {
_removeDuplicates(onDevice);
// _removeDuplicates sorts `onDevice` by checksum
final (toAdd, toUpdate, toDelete) = _diffAssets(onDevice, inDb);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the issue:
You compute a diff between the entire DB and a small batch from assets on device. The diff mechanism can't work that way. You'd either need to accumulate all device assets first (loosing the stream functionality for syncing...) OR you'd need to select the appropriate portion from the DB to compare to (which is challenging to get right, but possibly!)

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.

None yet

2 participants