[services][blob] Scaffold gRPC server
ClosedPublic
Actions

Authored by bartek on Nov 18 2022, 8:23 AM.

Details

Reviewers

varun
• jon
tomek
marcin

Commits

rCOMMdd91ab809771: [services][blob] Scaffold gRPC server

Summary

Boilerplate for proto interface. The server listens on 50051 port,
this is the default in current C++ implementation for all services. In subsequent diffs
I'll add possibility to configure this.

Depends on D5676

Test Plan

Run the service. Client should be able to send requests, server should send
"unimplemented" error messages to client.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bartek created this revision.Nov 18 2022, 8:23 AM

bartek held this revision as a draft.

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptNov 18 2022, 8:23 AM

bartek added reviewers: tomek, marcin.Nov 18 2022, 8:25 AM

Harbormaster completed remote builds in B13560: Diff 18579.Nov 18 2022, 8:33 AM

bartek added a child revision: D5678: [services][blob] Add AWS SDK config.Nov 18 2022, 9:03 AM

bartek published this revision for review.Nov 18 2022, 9:23 AM

varun accepted this revision.Nov 18 2022, 12:04 PM

This revision is now accepted and ready to land.Nov 18 2022, 12:04 PM

tomek requested changes to this revision.Nov 22 2022, 6:05 AM

tomek added inline comments.

services/blob/src/main.rs
14 ↗	(On Diff #18579)	Shouldn't we use a logging lib instead of println?
services/blob/src/service.rs
2 ↗	(On Diff #18579)	Where is this defined? It doesn't seem to be a part of this stack.
23 ↗	(On Diff #18579)	We can probably use `unimplemented!` macro

This revision now requires changes to proceed.Nov 22 2022, 6:05 AM

Does Phabricator require me to re-request reviews every time I reply to comments?

services/blob/src/main.rs
14 ↗	(On Diff #18579)	I'm planning to do it later - There's a Linear task for that: https://linear.app/comm/issue/ENG-2308/improve-logging-in-blob-service
services/blob/src/service.rs
2 ↗	(On Diff #18579)	It is generated from proto.
23 ↗	(On Diff #18579)	Doing it this way allowed me to test if responses are sent to the client.

If the revision shows up as "Needs Revision", that means it's on your queue on not on your reviewers' queue. If you update the diff you'll see that the status switches. Simply adding a comment will not switch the status, so yes you will need to re-request review if you don't think any of your reviewers' comments need to be addressed. This scenario should ideally be rare

bartek requested review of this revision.Nov 22 2022, 8:18 AM

varun accepted this revision.Nov 22 2022, 9:43 AM

tomek accepted this revision.Nov 23 2022, 3:29 AM

This revision is now accepted and ready to land.Nov 23 2022, 3:29 AM

Rebase

Harbormaster completed remote builds in B13719: Diff 18799.Nov 23 2022, 11:55 PM

Closed by commit rCOMMdd91ab809771: [services][blob] Scaffold gRPC server (authored by bartek). · Explain WhyNov 29 2022, 5:15 AM

This revision was automatically updated to reflect the committed changes.

bartek added a commit: rCOMMdd91ab809771: [services][blob] Scaffold gRPC server.