Page MenuHomePhabricator

[services] Backup - Add append holder reactor
AbandonedPublic

Authored by karol on Apr 19 2022, 1:56 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Oct 15, 6:29 PM
Unknown Object (File)
Sun, Oct 13, 3:06 PM
Unknown Object (File)
Sun, Oct 13, 3:06 PM
Unknown Object (File)
Sun, Oct 13, 2:58 PM
Unknown Object (File)
Sun, Oct 6, 9:44 AM
Unknown Object (File)
Sep 19 2024, 10:28 PM
Unknown Object (File)
Sep 13 2024, 3:27 AM
Unknown Object (File)
Sep 3 2024, 10:47 AM

Details

Reviewers
tomek
ashoat
Summary

Depends on D3773

Adding BlobAppendHolderClientReactor to the backup service

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

Test Plan
cd services
yarn run-backup-service

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

I suspect it probably would've been best to hold off on putting these diffs up... it feels like you just made three commits and diffed them immediately, but I suspect we will see a lot more commits you make to these files and subsequently a lot more diffs...

A good way to think about it: make sure you have a way to test something before you put up a diff up. In this case, looking at your test plans it seems like you don't have anything other than simply compiling the service.

tomek requested changes to this revision.Apr 21 2022, 2:12 AM

Is there a way to create a test plan that verifies the logic?

This revision now requires changes to proceed.Apr 21 2022, 2:12 AM

Kind of hard to come up with a different test plan than a successful compilation in a case when this code's not used anywhere. This is used in D3777 where the test plan points to D3535 and covers the client-server communication.

In D3774#105264, @karol-bisztyga wrote:

Kind of hard to come up with a different test plan than a successful compilation in a case when this code's not used anywhere. This is used in D3777 where the test plan points to D3535 and covers the client-server communication.

Ok, that makes sense. I find it useful to sometimes write new code just for the testing purpose - that might help. This code is usually really ugly, so it's costly to transform it into proper tests, but it lets me verify the correctness quickly.

This revision is now accepted and ready to land.Apr 21 2022, 8:37 AM
ashoat requested changes to this revision.Apr 21 2022, 3:16 PM

See feedback in D3772

This revision now requires changes to proceed.Apr 21 2022, 3:16 PM

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.