Page MenuHomePhabricator

[services] Backup - Blob Get Client - Add base structure
ClosedPublic

Authored by karol on Sep 1 2022, 4:08 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 20, 3:12 PM
Unknown Object (File)
Sat, Nov 16, 1:29 PM
Unknown Object (File)
Sun, Nov 10, 6:57 AM
Unknown Object (File)
Sun, Nov 3, 7:01 AM
Unknown Object (File)
Tue, Oct 29, 5:38 PM
Unknown Object (File)
Sun, Oct 27, 8:11 PM
Unknown Object (File)
Sun, Oct 27, 8:11 PM
Unknown Object (File)
Sun, Oct 27, 8:11 PM

Details

Summary

Depends on D5012

Adding base structure to the blob get client.

Test Plan

backup builds

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

karol edited the test plan for this revision. (Show Details)
karol added reviewers: tomek, varun, max.
tomek requested changes to this revision.Sep 5 2022, 5:26 AM
tomek added inline comments.
services/backup/blob_client/src/get_client.rs
25–26 ↗(On Diff #16173)

It was mentioned previously, but do we need an Arc here?

32–59 ↗(On Diff #16173)

I've seen somewhere in this stack the code, that was (almost?) exactly the same. Can we avoid the duplication?

services/backup/blob_client/src/lib.rs
26 ↗(On Diff #16173)

Can we avoid unsafe at function level?

This revision now requires changes to proceed.Sep 5 2022, 5:26 AM
karol added inline comments.
services/backup/blob_client/src/get_client.rs
25–26 ↗(On Diff #16173)
32–59 ↗(On Diff #16173)

Duplication is avoided in D5030 D5031

services/backup/blob_client/src/lib.rs
26 ↗(On Diff #16173)

If we want to receive the argument which is char* then rust will complain to make the function unsafe explicitly so I do not think so.

remove comment

tomek added inline comments.
services/backup/blob_client/src/get_client.rs
32–41 ↗(On Diff #16351)

I know this is refactored later, but we should use here the same approach as in D5006

This revision is now accepted and ready to land.Sep 7 2022, 5:06 AM
This revision was landed with ongoing or failed builds.Sep 8 2022, 4:10 AM
This revision was automatically updated to reflect the committed changes.