• karol retitled this revision from [services] Blob - Update service implementation to [draft] [services] Blob - Update service implementation.Mar 21 2022, 3:18 AM

Harbormaster completed remote builds in B7492: Diff 10547.Mar 21 2022, 3:20 AM

• karol requested review of this revision.Mar 21 2022, 3:20 AM

• karol added a child revision: D3487: [services] Backup - Fix copying big data chunk.Mar 22 2022, 1:25 AM

• karol edited the summary of this revision. (Show Details)Mar 24 2022, 2:26 AM

• karol added a parent revision: D3521: [services] Blob - Add server reactor implementations - put reactor.

tomek accepted this revision.Mar 25 2022, 9:06 AM

tomek added inline comments.

services/blob/docker-server/contents/server/src/BlobServiceImpl.cpp
101–103 ↗	(On Diff #10547)	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

tomek mentioned this in D3521: [services] Blob - Add server reactor implementations - put reactor.Mar 25 2022, 9:07 AM

• karol added inline comments.Mar 28 2022, 1:58 AM

services/blob/docker-server/contents/server/src/BlobServiceImpl.cpp
101–103 ↗	(On Diff #10547)	This is how unary reactors should be handled in the grpc async API, reference: https://github.com/grpc/grpc/blob/5a2422f42ac7f68be486bd8d9bd68458761419b6/examples/cpp/route_guide/route_guide_callback_server.cc#L91

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.

• karol mentioned this in D3467: [services] Backup - Add server reactor implementations - create new backup reactor.Mar 28 2022, 6:08 AM

tomek accepted this revision.Mar 30 2022, 4:27 AM

This revision is now accepted and ready to land.Mar 30 2022, 4:27 AM

tomek added inline comments.Mar 30 2022, 4:30 AM

services/blob/docker-server/contents/server/src/BlobServiceImpl.cpp
101–103 ↗	(On Diff #10547)	It's surprising that we need to get a default reactor and call Finish on it instead of calling `this->Finish()`

• karol added inline comments.Apr 1 2022, 3:28 AM

services/blob/docker-server/contents/server/src/BlobServiceImpl.cpp
101–103 ↗	(On Diff #10547)	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.