Paths

Table of Contentst

Differential D5013

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

Authored by • karol on Sep 1 2022, 11:08 AM.

Tags

None

Referenced Files

None

Subscribers

Details

Reviewers

• jon
tomek
varun
• max

Commits

rCOMM77d843164f0b: [services] Backup - Blob Get Client - Add base structure

Summary

Depends on D5012

Adding base structure to the blob get client.

Test Plan

backup builds

Diff Detail

Repository

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• karol created this revision.Sep 1 2022, 11:08 AM

Herald added a reviewer: • jon. · View Herald TranscriptSep 1 2022, 11:08 AM

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

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

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

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

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

Harbormaster completed remote builds in B11770: Diff 16173.Sep 1 2022, 11:24 AM

• karol requested review of this revision.Sep 1 2022, 11:24 AM

• karol added a child revision: D5014: [services] Backup - Blob Get Client - Add initialize.Sep 1 2022, 11:24 AM

tomek requested changes to this revision.Sep 5 2022, 12:26 PM

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, 12:26 PM

• karol requested review of this revision.Sep 6 2022, 8:43 AM

• karol added inline comments.

services/backup/blob_client/src/get_client.rs
25–26 ↗	(On Diff #16173)	https://linear.app/comm/issue/ENG-1732/remove-arc-from-lazy-static
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

remove comment

Harbormaster completed remote builds in B11908: Diff 16351.Sep 6 2022, 8:56 AM

tomek accepted this revision.Sep 7 2022, 12:06 PM

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, 12:06 PM

rebase

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

Closed by commit rCOMM77d843164f0b: [services] Backup - Blob Get Client - Add base structure. · Explain Why

This revision was automatically updated to reflect the committed changes.

• karol added a commit: rCOMM77d843164f0b: [services] Backup - Blob Get Client - Add base structure.

Harbormaster completed remote builds in B11990: Diff 16445.Sep 8 2022, 11:14 AM

Revision Contents
Changeset List

Path

Size

services/

backup/

blob_client/

src/

72 lines

9 lines

Diff 16445

services/backup/blob_client/src/get_client.rs

Loading...

services/backup/blob_client/src/lib.rs

Loading...