Page MenuHomePhabricator

[services] Backup - Blob Get Client - Add blocking read
ClosedPublic

Authored by karol on Sep 1 2022, 4:09 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 19, 2:11 PM
Unknown Object (File)
Mon, Mar 17, 10:21 PM
Unknown Object (File)
Sun, Mar 16, 12:23 AM
Unknown Object (File)
Thu, Mar 6, 6:03 AM
Unknown Object (File)
Thu, Mar 6, 5:52 AM
Unknown Object (File)
Tue, Mar 4, 5:13 AM
Unknown Object (File)
Tue, Mar 4, 5:13 AM
Unknown Object (File)
Tue, Mar 4, 5:13 AM

Details

Summary

Depends on D5014

Adding blocking read function to the blob get client.

Test Plan

backup builds

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jon requested changes to this revision.Sep 1 2022, 9:11 AM
jon added inline comments.
services/backup/blob_client/src/get_client.rs
131–137 ↗(On Diff #16175)

we pulled client out of the mutex guard, and now we are re-assigning it? why?

132–136 ↗(On Diff #16175)

This should be a similar way to express this.

This revision now requires changes to proceed.Sep 1 2022, 9:11 AM
services/backup/blob_client/src/get_client.rs
131–137 ↗(On Diff #16175)

Rust will not allow modifying struct members so we have to take() and re-put again.

132–136 ↗(On Diff #16175)

Yes, looks okay, nice catch, thanks!

tomek requested changes to this revision.Sep 5 2022, 5:37 AM
tomek added inline comments.
services/backup/blob_client/src/get_client.rs
131–137 ↗(On Diff #16175)

I'm not sure if this is correct in general. For example, you can use &mut reference...

135 ↗(On Diff #16175)

Is there a way to avoid mutating the data from outside this block? Can we use block_on return value for this?

tomek added inline comments.
services/backup/blob_client/src/get_client.rs
135 ↗(On Diff #16175)

Nice!

This revision is now accepted and ready to land.Sep 7 2022, 11:16 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.