Real-time Sync: Add base traits/interfaces #508
Conversation
hydra-yse
force-pushed
the
yse-rt-sync-base
branch
5 times, most recently
from
9ba2d75
to
503835e
Compare
We ensure that the prost-generated files match those which have been committed
hydra-yse
force-pushed
the
yse-rt-sync-base
branch
from
503835e
to
d658334
Compare
| async fn get_changes_since(&self, record_index: u64) -> Result<Vec<Record>>; | ||
|
|
||
| /// Adds a record to the remote | ||
| async fn set_record(&self, data: SyncData) -> Result<()>; |
There was a problem hiding this comment.
Since we are using a list of Records as response to get_changes_since I think we should stick to that structure also on set_record. SyncData seems a higher abstraction.
There was a problem hiding this comment.
That's actually been changed in #509, see here. I opted for having get_changes_since also decode the data.
Also, the reason why set_record does not take a record itself is because we populate the id and version fields automatically:
idgets the latestRecordId from the local databaseversionis the current schema verison
There was a problem hiding this comment.
@hydra-yse it is hard to review this PR by looking at changes in another PR.
I think either merge both into one PR or move the fixes that are related here to this PR.
There was a problem hiding this comment.
Sure, I'll merge the new interface definition here.
There was a problem hiding this comment.
The fact that we have a trait with set_record & get_changes_since means we have another implementation that will use these which will be triggered by the app. I think we need to see that implementation before we can determine if we need the interface to look like this.
I am OK with merging both PRs together, move to the implementation of the sync itself (at least partial) so we can review that effectively.
Closes #501