Page MenuHomePhabricator
Differential D5099

[services] Backup - Blob Get Client - Pass rust string instead of const char*
ClosedPublic
Actions

Authored by • karol on Sep 9 2022, 7:07 AM.
Tags
None
Referenced Files
F3365801: D5099.id16617.diff
Mon, Nov 25, 7:48 AM
F3365732: D5099.id16556.diff
Mon, Nov 25, 7:31 AM
F3364769: D5099.diff
Mon, Nov 25, 5:07 AM
Unknown Object (File)
Fri, Nov 22, 6:53 AM
Unknown Object (File)
Thu, Nov 21, 10:30 PM
Unknown Object (File)
Thu, Nov 14, 5:06 PM
Unknown Object (File)
Sun, Nov 10, 6:57 AM
Unknown Object (File)
Thu, Oct 31, 5:12 AM
View All Files
Subscribers
• abosh
ashoat
atul
tomek

Details

Reviewers
• jon
tomek
• max
varun
Commits
rCOMMfad989a99d6c: [services] Backup - Blob Get Client - Pass rust string instead of const char*
Summary

Depends on D5097

Raised in https://phab.comm.dev/D5035#148669

We can construct rust::Strings in c++ instead of passing const char*s from c++ to rust.

Test Plan

Services build, tests pass.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

• karol created this revision.Sep 9 2022, 7:07 AM
Herald added a reviewer: • jon. · View Herald TranscriptSep 9 2022, 7:07 AM
Herald added subscribers: • abosh, atul, tomek, ashoat. · View Herald Transcript
Harbormaster returned this revision to the author for changes because remote builds failed.Sep 9 2022, 7:08 AM
Harbormaster failed remote builds in B12055: Diff 16555!
• karol edited the summary of this revision. (Show Details)Sep 9 2022, 7:11 AM
• karol edited the test plan for this revision. (Show Details)
• karol added reviewers: tomek, • max, varun.
• karol added a parent revision: D5098: [services] Rust Integration - Backup - Rust - Fix backup utils.
• karol mentioned this in D5035: [services] Rust Integration - Backup - Rust - Multiple put clients.
• karol updated this revision to Diff 16556.Sep 9 2022, 7:12 AM

retrigger CI - failed on identity build that's unrelated

Harbormaster completed remote builds in B12056: Diff 16556.Sep 9 2022, 7:22 AM
• karol requested review of this revision.Sep 9 2022, 7:22 AM
tomek requested changes to this revision.Sep 9 2022, 7:30 AM
tomek added inline comments.
services/backup/blob_client/src/lib.rs
17–39 ↗(On Diff #16556)

Are the functions still unsafe? Our guess was that it was caused by a raw pointer.

This revision now requires changes to proceed.Sep 9 2022, 7:30 AM
• karol added inline comments.Sep 9 2022, 7:41 AM
services/backup/blob_client/src/lib.rs
17–39 ↗(On Diff #16556)

Right, missed that.

• karol updated this revision to Diff 16576.Sep 12 2022, 1:40 AM

avoid unsafe

Harbormaster completed remote builds in B12076: Diff 16576.Sep 12 2022, 1:49 AM
• jon requested changes to this revision.Sep 12 2022, 9:11 AM
• jon added inline comments.
services/backup/blob_client/src/put_client.rs
200 ↗(On Diff #16576)

since we are just passing this to a hashmap, we should be able to use &str. Also avoids all the .clone() usage everywhere as you're no longer taking ownership of the string

This revision now requires changes to proceed.Sep 12 2022, 9:11 AM
• karol edited the summary of this revision. (Show Details)Sep 13 2022, 1:09 AM
• karol added a parent revision: D5097: [services] Backup - Blob Get Client - Cleanup.
• karol added inline comments.Sep 13 2022, 1:31 AM
services/backup/blob_client/src/put_client.rs
200 ↗(On Diff #16576)

Right, we can refactor some Strings into &strs, will do.

• karol updated this revision to Diff 16617.Sep 13 2022, 1:32 AM

use more &str instead of String

Harbormaster completed remote builds in B12110: Diff 16617.Sep 13 2022, 1:41 AM
• jon accepted this revision.Sep 13 2022, 12:12 PM

improvement

tomek accepted this revision.Sep 15 2022, 9:02 AM
This revision is now accepted and ready to land.Sep 15 2022, 9:02 AM
• karol mentioned this in D5089: [services] Backup - Blob Get Client - Prepare for error handling.Sep 15 2022, 9:09 AM
• karol updated this revision to Diff 16710.Sep 15 2022, 9:17 AM

remove tools

Harbormaster completed remote builds in B12182: Diff 16710.Sep 15 2022, 9:27 AM
• karol updated this revision to Diff 16847.Sep 19 2022, 7:27 AM

rebase

This revision was landed with ongoing or failed builds.Sep 19 2022, 7:28 AM
Closed by commit rCOMMfad989a99d6c: [services] Backup - Blob Get Client - Pass rust string instead of const char* (authored by • karol). · Explain Why
This revision was automatically updated to reflect the committed changes.
• karol added a commit: rCOMMfad989a99d6c: [services] Backup - Blob Get Client - Pass rust string instead of const char*.
Harbormaster completed remote builds in B12285: Diff 16847.Sep 19 2022, 7:45 AM

Revision Contents
Changeset List

PathSize
services/
backup/
blob_client/
src/
24 lines
27 lines
23 lines
src/
Reactors/
server/
16 lines
16 lines
2 lines
12 lines

Diff 16856

View Options

services/backup/blob_client/src/get_client.rs

Loading...
View Options

services/backup/blob_client/src/lib.rs

Loading...
View Options

services/backup/blob_client/src/put_client.rs

Loading...
View Options

services/backup/blob_client/src/tools.rs

Loading...
View Options

services/backup/src/Reactors/server/AddAttachmentsUtility.cpp

Loading...
View Options

services/backup/src/Reactors/server/CreateNewBackupReactor.cpp

Loading...
View Options

services/backup/src/Reactors/server/PullBackupReactor.cpp

Loading...
View Options

services/backup/src/Reactors/server/SendLogReactor.cpp

Loading...