Page MenuHomePhabricator

[backup client] Downloading logs
ClosedPublic

Authored by michal on Feb 7 2024, 9:44 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 19, 12:12 AM
Unknown Object (File)
Wed, Dec 18, 8:58 PM
Unknown Object (File)
Mon, Dec 9, 1:48 AM
Unknown Object (File)
Sun, Dec 8, 12:37 AM
Unknown Object (File)
Fri, Dec 6, 9:03 PM
Unknown Object (File)
Nov 20 2024, 2:54 PM
Unknown Object (File)
Nov 20 2024, 2:54 PM
Unknown Object (File)
Nov 8 2024, 4:50 AM
Subscribers

Details

Summary

Implemented a backup client function that handles downloading the logs, taking into account network failures, missing logs etc. and retrying the download in case of failure. The tests have been rewritten to use this new function. I considered handling the missing logs better (maybe try and batch them etc.) but because websocket connection is bsaed on TCP, this should be an issue (my testing also showed no problems with this).

Removed the code on native as it will be completely changed anyways in the future diffs.

Added tokio as a dependency, but we already use this exact version, and as you can see from the lock files, there is no actual change in the used dependencies (both commtest and native already depended on tokio).

Depends on D10985

Test Plan

Run the updated integration test. Also tested if the connection is retried in case of any errors.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

The download_logs() and log_download_stream() logic is unobvious at first glance but after offline discussions, I think they work as expected.

shared/backup_client/src/lib.rs
368 ↗(On Diff #36790)

I know this pain, derive_more::Error is so limited

This revision is now accepted and ready to land.Feb 9 2024, 7:52 AM