Get back to Recovering syncing when we haven't sync for a while #3995
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3995 +/- ##
=======================================
Coverage 84.23% 84.24%
=======================================
Files 266 266
Lines 28349 28364 +15
=======================================
+ Hits 23881 23894 +13
- Misses 4468 4470 +2 ☔ View full report in Codecov by Sentry. |
MatMaul
force-pushed
the
recovering
branch
3 times, most recently
from
4f08a1f
to
0b79ccb
Compare
| Running => Running, | ||
| Running { last_time } => { | ||
| // We haven't sync for a while so we should go back to recovering | ||
| if last_time.elapsed().as_secs() > 1800 { |
There was a problem hiding this comment.
Ideally, we would like to make this (1800) configurable because it can depend on the homeserver or how the user wants to use this API. Thoughts?
There was a problem hiding this comment.
If this is configurable, then we don't need mock_instant, nor the testing feature (we want to remove them, and this patch is adding them). If this is configurable then, we can simply set it to 1 sec, and have a sleep of 2 secs in the test. It can sound like a bit hacky but it's probably easier to maintain than having a new dependency and a new feature.
There was a problem hiding this comment.
I am not sure it will be used much but it doesn't cost much to make it a parameter, and if it helps removing some maintenance burden in the tests it's probably worth it.
Yeah what you propose for the test is a bit hacky but it works ™️ , and it feels fine at the integration level.
On it.
There was a problem hiding this comment.
Some docs are still missing, I'll clean up later on in the week, probably from Berlin ;)
Fixes #3935.
Untested in the apps for now, but integration test is included and working.
Signed-off-by: Mathieu Velten mathieu@velten.xyz