ENG-6213 : Create long lived backup client
Implemented a native rust backup client that creates a websocket connection for logs, keeps it alive and retries when getting an error.
Details
Details
Tested on Android and iOS:
- Started the backup handler -> checked the backup service logs for new websocket connection
- Waited a bit to make sure ping and pongs are handled correctly
- Killed the backup service and restarted it -> the connectino was restarted automatically
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
Nothing wrong with the code, I just want to understand it better before accepting, left some questions. Feel free to re-request
| native/native_rust_library/src/backup.rs | ||
|---|---|---|
| 28–46 ↗ | (On Diff #35609) | One thing I don't like about code organization here is that:
Also, can you briefly explain (from the sync/async/threading perspective) why one function is doing the lock().take() logic directly and then spawns a RUNTIME task, while the other does this inside that task? |
| 41 ↗ | (On Diff #35609) | why is it called _sync while it spawns an async task and returns a promise? |
| 118–120 ↗ | (On Diff #35609) |
|
Comment Actions
Fixes
| native/native_rust_library/src/backup.rs | ||
|---|---|---|
| 28–46 ↗ | (On Diff #35609) | I have removed the async from the stop handler function and now it works like the start handler function. The reason it looked like this is because when I was implementing this code, the stop_backup_handler could optionally wait for all data to be uploaded and this required some async operations (e.g. checking if all files were uploaded). I decided to move this logic to a later diff but the general structure remained (but it's now fixed). |
| 41 ↗ | (On Diff #35609) | This has been a naming convention I have been using for async functions exposed to cxx (because they can't be exposed directly).
for some identity functions we use fn xxx() and async fn xxx_helper() convention but I'm not a fan of that. |
| 118–120 ↗ | (On Diff #35609) | 1,2 fixed. I will make a task for 3 |