Page MenuHomePhabricator

[services] Blob - Update service implementation
ClosedPublic

Authored by karol on Mar 21 2022, 2:40 AM.
Tags
None
Referenced Files
F3377152: D3471.diff
Wed, Nov 27, 4:08 AM
Unknown Object (File)
Sat, Nov 23, 2:56 PM
Unknown Object (File)
Tue, Nov 19, 11:37 PM
Unknown Object (File)
Fri, Nov 15, 11:24 PM
Unknown Object (File)
Thu, Nov 14, 11:03 PM
Unknown Object (File)
Wed, Nov 13, 1:08 AM
Unknown Object (File)
Tue, Nov 5, 6:15 PM
Unknown Object (File)
Thu, Oct 31, 9:35 PM

Details

Summary

Depends on D3521

Update implementation of 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

karol retitled this revision from [services] Blob - Update service implementation to [draft] [services] Blob - Update service implementation.Mar 21 2022, 3:18 AM
tomek added inline comments.
services/blob/docker-server/contents/server/src/BlobServiceImpl.cpp
101–103

Could you explain what is the purpose of this code?

This revision is now accepted and ready to land.Mar 25 2022, 9:06 AM
services/blob/docker-server/contents/server/src/BlobServiceImpl.cpp
101–103
karol retitled this revision from [draft] [services] Blob - Update service implementation to [services] Blob - Update service implementation.

It is accepted as a draft so I removed the draft label and I'm requesting changes once again to keep the appropriate order of events.

This revision is now accepted and ready to land.Mar 30 2022, 4:27 AM
services/blob/docker-server/contents/server/src/BlobServiceImpl.cpp
101–103

It's surprising that we need to get a default reactor and call Finish on it instead of calling this->Finish()

services/blob/docker-server/contents/server/src/BlobServiceImpl.cpp
101–103

I don't understand your logic, sorry. this->Finish would indicate that we want to finish the entire connection since this refers to the BlobServiceImpl.
The already prepared reactor seems ok, this is a common task for a one-arg connection and we return a new instance of a reactor like for every other type of connection.

services/blob/docker-server/contents/server/src/BlobServiceImpl.cpp
101–103

Ah, ok, that makes sense