Page MenuHomePhabricator

[backup-client] Uploading logs
ClosedPublic

Authored by michal on Jan 4 2024, 7:27 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Jul 3, 4:59 AM
Unknown Object (File)
Sun, Jun 30, 12:44 AM
Unknown Object (File)
Sat, Jun 29, 2:10 PM
Unknown Object (File)
Tue, Jun 25, 11:07 PM
Unknown Object (File)
Sun, Jun 23, 12:39 AM
Unknown Object (File)
Sat, Jun 22, 8:15 AM
Unknown Object (File)
Sat, Jun 22, 8:15 AM
Unknown Object (File)
Sat, Jun 22, 8:15 AM
Subscribers

Details

Summary

ENG-5557 : Add support for uploading logs from backup clients

This diff adds an option to create a websocket connection, upload logs and receive upload confirmations from the backup service. The stream/sink from tungstenite is mapped so that the clients only deal with business logic. The reconnection etc. will be handled in the outside code.

Depends on D10457

Test Plan

Tested with next diffs from commtest and native mobile devices. Tested with this code:

let (tx, rx) = backup_client
  .upload_logs(&user_identity, &backup_data.backup_id)
  .await
  .unwrap();

tokio::pin!(tx);
tokio::pin!(rx);

for log_data in log_datas {
  tx.send(log_data.clone()).await.unwrap();
}

let result: HashSet<usize> =
  rx.take(log_datas.len()).try_collect().await.unwrap();
let expected = log_datas.iter().map(|data| data.log_id).collect();
assert_eq!(result, expected);

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

shared/backup_client/Cargo.toml
18–20 ↗(On Diff #35199)

All of these dependencies (with these specific versions) are already used elsewhere in our codebase.

michal requested review of this revision.Jan 4 2024, 7:45 AM
bartek added inline comments.
shared/backup_client/src/lib.rs
138–142 ↗(On Diff #35199)

Does this block have to be async if there are no async calls inside?

161–164 ↗(On Diff #35199)

You probably don't need returns here

This revision is now accepted and ready to land.Jan 5 2024, 3:08 AM

Remove return

shared/backup_client/src/lib.rs
138–142 ↗(On Diff #35199)

Yes, with expects a future. An alternative would be to use a ready future but I think using async is cleaner.

This revision was landed with ongoing or failed builds.Jan 8 2024, 7:20 AM
This revision was automatically updated to reflect the committed changes.