Page MenuHomePhabricator
Differential D3773

[services] Blob - Add append holder implementation
AbandonedPublic
Actions

Authored by • karol on Apr 19 2022, 1:56 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 16, 4:29 PM
Unknown Object (File)
Sat, Nov 16, 4:29 PM
Unknown Object (File)
Sat, Nov 16, 4:29 PM
Unknown Object (File)
Tue, Nov 5, 5:10 PM
Unknown Object (File)
Oct 13 2024, 3:06 PM
Unknown Object (File)
Oct 13 2024, 3:06 PM
Unknown Object (File)
Oct 13 2024, 2:58 PM
Unknown Object (File)
Oct 5 2024, 9:33 PM
View All Files
Subscribers
• abosh
• adrian
ashoat
atul
• benschac
tomek

Details

Reviewers
tomek
ashoat
Summary

Depends on D3772

Adding AppendHolder to the blob service implementation.

https://linear.app/comm/issue/ENG-672/create-addreferencetohashbyholder-in-the-blob-service

Test Plan
cd services
yarn run-blob-service

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• karol created this revision.Apr 19 2022, 1:56 AM
Herald added subscribers: • abosh, • benschac, atul and 3 others. · View Herald TranscriptApr 19 2022, 1:56 AM
• karol edited the summary of this revision. (Show Details)Apr 19 2022, 2:14 AM
• karol edited the test plan for this revision. (Show Details)
• karol added a reviewer: tomek.
• karol added a parent revision: D3772: [services] Blob - Add append holder to proto.
Harbormaster completed remote builds in B8295: Diff 11591.Apr 19 2022, 2:15 AM
• karol requested review of this revision.Apr 19 2022, 2:15 AM
• karol added a child revision: D3774: [services] Backup - Add append holder reactor.Apr 19 2022, 2:17 AM
tomek accepted this revision.Apr 21 2022, 2:03 AM

This diff depends on an ongoing proto discussion - this is really inefficient. We should put up a diff with .proto / arch review way before starting the implementation. The current pattern makes it expensive to do any significant changes.

Overall it looks ok!

This revision is now accepted and ready to land.Apr 21 2022, 2:03 AM
ashoat requested changes to this revision.Apr 21 2022, 1:35 PM

I'd rather not have this accepted until D3772 is... I fear @karol-bisztyga will land it as-is and then submit another follow-up diff to modify it after he gets changes requested on D3772

This revision now requires changes to proceed.Apr 21 2022, 1:35 PM
• karol added a comment.Apr 22 2022, 2:00 AM
In D3773#105581, @ashoat wrote:

I'd rather not have this accepted until D3772 is... I fear @karol-bisztyga will land it as-is and then submit another follow-up diff to modify it after he gets changes requested on D3772

I wouldn't land a diff that has at least one unaccepted predecessor but I understand your concern.

• karol abandoned this revision.Apr 22 2022, 3:27 AM

This is a part of mistakenly added code, more details in https://phabricator.ashoat.com/D3772#105661.

Abandoning. Sorry for the inconvenience, thank you for the review.

Revision Contents
Changeset List

PathSize
services/
blob/
src/
4 lines
25 lines

Diff 11591

View Options

services/blob/src/BlobServiceImpl.h

Loading...
View Options

services/blob/src/BlobServiceImpl.cpp

Loading...