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
Unknown Object (File)
Tue, Mar 18, 1:43 AM
Unknown Object (File)
Mon, Mar 17, 4:07 AM
Unknown Object (File)
Mon, Mar 17, 4:07 AM
Unknown Object (File)
Mon, Mar 17, 4:07 AM
Unknown Object (File)
Sun, Mar 16, 12:39 AM
Unknown Object (File)
Fri, Mar 14, 6:32 AM
Unknown Object (File)
Tue, Mar 11, 2:46 PM
Unknown Object (File)
Sun, Mar 9, 12:04 PM
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

Show All 31 Lines.questionText {
color: var(--white-100); color: var(--white-100);
} }
.icon { .icon {
margin-left: 24px; margin-left: 24px;
} }
.answerContainer { .answerContainer {
max-height: 0; display: grid;
overflow: hidden; grid-template-rows: 0fr;
transition: grid-template-rows 500ms;
} }
.activeAnswerContainer { .activeAnswerContainer {
max-height: fit-content; grid-template-rows: 1fr;
} }
.answerText { .answerText {
color: var(--white-100); color: var(--white-100);
margin-top: 16px; margin-top: 16px;
overflow: hidden;
} }
.link { .link {
color: inherit; color: inherit;
text-decoration: underline; text-decoration: underline;
transition: color 150ms; transition: color 150ms;
} }
.link:hover { .link:hover {
color: var(--violet-dark-100); color: var(--violet-dark-100);
transition: color 150ms; transition: color 150ms;
} }
View Options

services/backup/blob_client/src/lib.rs

Show All 31 Lines.questionText {
color: var(--white-100); color: var(--white-100);
} }
.icon { .icon {
margin-left: 24px; margin-left: 24px;
} }
.answerContainer { .answerContainer {
max-height: 0; display: grid;
overflow: hidden; grid-template-rows: 0fr;
transition: grid-template-rows 500ms;
} }
.activeAnswerContainer { .activeAnswerContainer {
max-height: fit-content; grid-template-rows: 1fr;
} }
.answerText { .answerText {
color: var(--white-100); color: var(--white-100);
margin-top: 16px; margin-top: 16px;
overflow: hidden;
} }
.link { .link {
color: inherit; color: inherit;
text-decoration: underline; text-decoration: underline;
transition: color 150ms; transition: color 150ms;
} }
.link:hover { .link:hover {
color: var(--violet-dark-100); color: var(--violet-dark-100);
transition: color 150ms; transition: color 150ms;
} }
View Options

services/backup/blob_client/src/put_client.rs

Show All 31 Lines.questionText {
color: var(--white-100); color: var(--white-100);
} }
.icon { .icon {
margin-left: 24px; margin-left: 24px;
} }
.answerContainer { .answerContainer {
max-height: 0; display: grid;
overflow: hidden; grid-template-rows: 0fr;
transition: grid-template-rows 500ms;
} }
.activeAnswerContainer { .activeAnswerContainer {
max-height: fit-content; grid-template-rows: 1fr;
} }
.answerText { .answerText {
color: var(--white-100); color: var(--white-100);
margin-top: 16px; margin-top: 16px;
overflow: hidden;
} }
.link { .link {
color: inherit; color: inherit;
text-decoration: underline; text-decoration: underline;
transition: color 150ms; transition: color 150ms;
} }
.link:hover { .link:hover {
color: var(--violet-dark-100); color: var(--violet-dark-100);
transition: color 150ms; transition: color 150ms;
} }
View Options

services/backup/blob_client/src/tools.rs

Show All 31 Lines.questionText {
color: var(--white-100); color: var(--white-100);
} }
.icon { .icon {
margin-left: 24px; margin-left: 24px;
} }
.answerContainer { .answerContainer {
max-height: 0; display: grid;
overflow: hidden; grid-template-rows: 0fr;
transition: grid-template-rows 500ms;
} }
.activeAnswerContainer { .activeAnswerContainer {
max-height: fit-content; grid-template-rows: 1fr;
} }
.answerText { .answerText {
color: var(--white-100); color: var(--white-100);
margin-top: 16px; margin-top: 16px;
overflow: hidden;
} }
.link { .link {
color: inherit; color: inherit;
text-decoration: underline; text-decoration: underline;
transition: color 150ms; transition: color 150ms;
} }
.link:hover { .link:hover {
color: var(--violet-dark-100); color: var(--violet-dark-100);
transition: color 150ms; transition: color 150ms;
} }
View Options

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

Show All 31 Lines.questionText {
color: var(--white-100); color: var(--white-100);
} }
.icon { .icon {
margin-left: 24px; margin-left: 24px;
} }
.answerContainer { .answerContainer {
max-height: 0; display: grid;
overflow: hidden; grid-template-rows: 0fr;
transition: grid-template-rows 500ms;
} }
.activeAnswerContainer { .activeAnswerContainer {
max-height: fit-content; grid-template-rows: 1fr;
} }
.answerText { .answerText {
color: var(--white-100); color: var(--white-100);
margin-top: 16px; margin-top: 16px;
overflow: hidden;
} }
.link { .link {
color: inherit; color: inherit;
text-decoration: underline; text-decoration: underline;
transition: color 150ms; transition: color 150ms;
} }
.link:hover { .link:hover {
color: var(--violet-dark-100); color: var(--violet-dark-100);
transition: color 150ms; transition: color 150ms;
} }
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...