Page MenuHomePhabricator

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

Authored by karol on Sep 1 2022, 4:07 AM.
Tags
None
Referenced Files
F3342350: D5010.diff
Fri, Nov 22, 1:20 AM
Unknown Object (File)
Sat, Nov 16, 1:29 PM
Unknown Object (File)
Sat, Nov 16, 8:15 AM
Unknown Object (File)
Sat, Nov 16, 8:13 AM
Unknown Object (File)
Sun, Nov 10, 6:57 AM
Unknown Object (File)
Wed, Oct 30, 11:28 PM
Unknown Object (File)
Sun, Oct 27, 8:11 PM
Unknown Object (File)
Sun, Oct 27, 8:11 PM

Details

Summary

Depends on D5009

Adding blocking read function to the blob put client.

Test Plan

backup builds

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek requested changes to this revision.Sep 5 2022, 5:12 AM
tomek added inline comments.
services/backup/blob_client/src/put_client.rs
198–206 ↗(On Diff #16170)

Is it possible to avoid taking and setting back the client? Can we use references to achieve the same result but in a simpler way?

This revision now requires changes to proceed.Sep 5 2022, 5:12 AM
karol added inline comments.
services/backup/blob_client/src/put_client.rs
198–206 ↗(On Diff #16170)

I'm not sure, I already spent some time trying to hack this and by far that's the only way I could make it work. This was raised across my diffs several times already. I don't see it that harmful at the end of the day, not sure what's the big deal here.

tomek added inline comments.
services/backup/blob_client/src/put_client.rs
198–206 ↗(On Diff #16170)

Ok, let's treat this as a followup.

200 ↗(On Diff #16170)

I've mentioned this in other places, but it would be great if we could return this value from async block instead of mutating a response variable.

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