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
PE-2618: Sync from Snapshots #826
Conversation
…Composite PE-2631
PE-2632: Make use of the components for Quick Sync [Read-side]
Visit the preview URL for this PR (updated for commit 1b5ba34): https://ardrive-web--pr826-pe-2618-ltumutwi.web.app (expires Wed, 04 Jan 2023 18:52:51 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: a224ebaee2f0939e7665e7630e7d3d6cd7d0f8b0 |
@@ -6,13 +6,15 @@ Stream<double> _parseDriveTransactionsIntoDatabaseEntities({ | |||
required DriveDao driveDao, | |||
required Database database, | |||
required ArweaveService arweaveService, | |||
required List<DriveEntityHistory$Query$TransactionConnection$TransactionEdge> | |||
required List< |
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.
We need a better way to handle these. It's risky to rely on generated types like this without some abstraction.
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.
@@ -42,15 +44,20 @@ Stream<double> _parseDriveTransactionsIntoDatabaseEntities({ | |||
final owner = await arweave.getOwnerForDriveEntityWithId(drive.id); | |||
|
|||
yield* _batchProcess< | |||
DriveEntityHistory$Query$TransactionConnection$TransactionEdge>( | |||
DriveEntityHistory$Query$TransactionConnection$TransactionEdge$Transaction>( |
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.
Maybe we should create a tech debt ticket for a better GQL abstraction in the future.
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.
PE-2782 🚀
359384f
lib/blocs/sync/utils/sync_drive.dart
Outdated
lastBlockHeight, | ||
) | ||
.map( | ||
(edge) => edge.node, |
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.
Perhaps move this mapping inside the function to abstract it away. The sync cubit shouldn't need to know about edges or nodes.
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.
Much cleaner :) - ae51d75
lib/blocs/sync/utils/sync_drive.dart
Outdated
await for (var t in transactionsStream) { | ||
if (t.isEmpty) continue; | ||
|
||
await for (DriveEntityHistory$Query$TransactionConnection$TransactionEdge$Transaction t |
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.
Simplify this for now by adding to the start of the file.
typedef DriveEntityTransaction = DriveEntityHistory$Query$TransactionConnection$TransactionEdge$Transaction;
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.
} | ||
|
||
String get _dataUri { | ||
return 'https://arweave.net/$txId'; |
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.
We can get the url from config instead.
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.
Like so? - b3ac93e
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.
ConfigService and AppConfig already have it defined. You just need to use those.
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.
return _getNextStream(); | ||
} | ||
|
||
Stream<DriveEntityHistory$Query$TransactionConnection$TransactionEdge$Transaction> |
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.
These long types almost make me cry
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.
There you go, Javed 😭 - 359384f
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.
Fantastic work 💯
…ose only after finished PE-2618
Integration branch for the Story's sub-tasks.
--- Releases ---
Android release: https://appdistribution.firebase.google.com/testerapps/1:305132849030:android:6cf0cd5ec064fad3ffce07/releases/7unj2boou3560
iOS release: https://appdistribution.firebase.google.com/testerapps/1:305132849030:ios:06ea33a95a2e0d42ffce07/releases/56ec6b5bfal0g