Paths

Table of Contentst

Differential D5010

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

Authored by • karol on Sep 1 2022, 4:07 AM.

Tags

None

Referenced Files

	F2098962: D5010.id16442.diff
	Mon, Jun 24, 3:55 PM

	Unknown Object (File)
	Sat, Jun 22, 5:06 AM

	Unknown Object (File)
	Fri, Jun 21, 11:38 AM

	Unknown Object (File)
	Fri, Jun 21, 3:13 AM

	Unknown Object (File)
	Wed, Jun 19, 10:23 PM

	Unknown Object (File)
	Wed, Jun 19, 9:11 PM

	Unknown Object (File)
	Wed, Jun 19, 12:49 PM

	Unknown Object (File)
	Fri, Jun 14, 4:27 AM

View All 88 Files

Subscribers

Details

Reviewers

• jon
tomek
varun
• max

Commits

rCOMMc915b14cca4a: [services] Backup - Blob Put Client - Add blocking read

Summary

Depends on D5009

Adding blocking read function to the blob put client.

Test Plan

backup builds

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• karol created this revision.Sep 1 2022, 4:07 AM

Herald added a reviewer: • jon. · View Herald TranscriptSep 1 2022, 4:07 AM

Herald added subscribers: • abosh, atul, tomek, ashoat. · View Herald Transcript

Harbormaster completed remote builds in B11767: Diff 16170.Sep 1 2022, 4:20 AM

• karol requested review of this revision.Sep 1 2022, 4:20 AM

• karol edited the summary of this revision. (Show Details)Sep 1 2022, 4:22 AM

• karol edited the test plan for this revision. (Show Details)

• karol added reviewers: tomek, varun, • max.

• karol added a parent revision: D5009: [services] Backup - Blob Put Client - Add initialize.

• karol added a child revision: D5011: [services] Backup - Blob Put Client - Add write.

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 requested review of this revision.Sep 6 2022, 1:30 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.

• karol added inline comments.Sep 6 2022, 1:35 AM

services/backup/blob_client/src/put_client.rs
198–206 ↗	(On Diff #16170)	https://linear.app/comm/issue/ENG-1736/try-to-avoid-reassigning-when-modifying-the-field-of-a-global-struct

tomek accepted this revision.Sep 6 2022, 7:14 AM

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

address comment

Harbormaster completed remote builds in B11978: Diff 16433.Sep 8 2022, 3:36 AM

rebase

This revision was landed with ongoing or failed builds.Sep 8 2022, 4:10 AM

Closed by commit rCOMMc915b14cca4a: [services] Backup - Blob Put Client - Add blocking read (authored by • karol). · Explain Why

This revision was automatically updated to reflect the committed changes.

• karol added a commit: rCOMMc915b14cca4a: [services] Backup - Blob Put Client - Add blocking read.

Harbormaster completed remote builds in B11987: Diff 16442.Sep 8 2022, 4:12 AM

Revision Contents
Changeset List

Path

Size

services/

backup/

blob_client/

src/

23 lines

Diff 16468

services/backup/blob_client/src/put_client.rs

Loading...