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

Update Sync Interface #222

Closed
wants to merge 20 commits into from
Closed

Update Sync Interface #222

wants to merge 20 commits into from

Conversation

LiranCohen
Copy link
Member

This is a WIP but opening it up for some input and reviews.

Goals of this PR:

  • to simplify the SyncManager interface and make it easier to test
  • add support for testing multiple remote DWNs

This PR prepares the pull/push queues before calling sync, this will allow to test out different strategies for enqueuing syncs, and currently allows pull/push to be called async after enqueuing.

I have some ideas of changes to make to the DWN SDK/DWN Server to make sync more efficient, but will work through those in a separate PR.

Additionally, I am creating some chaos-monkey type scripts that will live in a separate repo. This will allow us to also compare syncing performance with different scenarios.

**** NOTES
I think we should enforce a minimum sync, however for certain tests this minimum should be low. I've added logic for the testing environments, but locally you currently have to pass a specific ENV variable for tests to pass. This is just temporary.

Looking for input on how best to handle this, or just remove the minimum interval from the code completely.

@codecov
Copy link

codecov bot commented Sep 19, 2023

Codecov Report

Merging #222 (1fb120f) into main (f3c7184) will increase coverage by 0.38%.
The diff coverage is 95.93%.

@@            Coverage Diff             @@
##             main     #222      +/-   ##
==========================================
+ Coverage   90.99%   91.37%   +0.38%     
==========================================
  Files          67       67              
  Lines       12645    12666      +21     
  Branches     1260     1292      +32     
==========================================
+ Hits        11506    11574      +68     
+ Misses       1116     1067      -49     
- Partials       23       25       +2     
Components Coverage Δ
api 94.32% <ø> (ø)
common 95.00% <ø> (ø)
credentials 92.77% <ø> (ø)
crypto 100.00% <ø> (ø)
dids 92.16% <ø> (ø)
agent 89.22% <95.93%> (+1.05%) ⬆️
identity-agent 59.05% <ø> (ø)
proxy-agent 58.59% <ø> (ø)
user-agent 57.36% <ø> (ø)

@LiranCohen LiranCohen force-pushed the lirancohen/sync-interface branch 3 times, most recently from d736c44 to 3659441 Compare September 29, 2023 16:13
@frankhinek
Copy link
Contributor

Talked to @LiranCohen about this PR yesterday and he's going to review and determine whether it should be closed or taken out of Draft state for review.

@frankhinek
Copy link
Contributor

@LiranCohen said he'll re-open later once ready for review

@frankhinek frankhinek closed this Jan 16, 2024
@LiranCohen LiranCohen deleted the lirancohen/sync-interface branch April 1, 2024 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants