Page MenuHomePhabricator

[services][identity] grpc scaffolding for the identity service
ClosedPublic

Authored by varun on Mar 30 2022, 4:03 PM.
Tags
None
Referenced Files
F3377482: D3578.id11130.diff
Wed, Nov 27, 5:56 AM
F3377405: D3578.id10849.diff
Wed, Nov 27, 5:30 AM
F3377181: D3578.id10915.diff
Wed, Nov 27, 4:16 AM
Unknown Object (File)
Sat, Nov 23, 1:09 PM
Unknown Object (File)
Sat, Nov 23, 12:11 PM
Unknown Object (File)
Sat, Nov 23, 12:10 PM
Unknown Object (File)
Sat, Nov 23, 12:10 PM
Unknown Object (File)
Wed, Nov 13, 1:36 AM

Details

Summary

added a bare-bones implementation of the identity service defined in identity.proto

Depends on D3573

Test Plan

ran cargo build && cargo run, then used BloomRPC to make calls to the server. got a successful response.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Mar 30 2022, 4:08 PM
Harbormaster failed remote builds in B7735: Diff 10849!

Looks like the CI automatically marked this as "Changes planned" because of the failed build. The build that failed is the android build and since this diff has nothing to do with android, the error is most probably irrelevant, and effectively the state's incorrect. I think it's good to manually request review in such cases so the reviewers can see the diff in their queue.

services/identity/build.rs
1 ↗(On Diff #10849)

so Box<dyn std::error::Error> is automatically returned via ? used in the next line?

services/identity/src/main.rs
26 ↗(On Diff #10849)

can we use 0.0.0.0 like in all other services?

50051 is a tunnelbroker's port, we probably want to use 50054 - https://github.com/CommE2E/comm/blob/2463ccbd032927571d7c7a968df199a1563a35d3/services/.env

what this parse does here?

32 ↗(On Diff #10849)

So this is just calling an argument-less function? Or why there are no ()?

varun requested review of this revision.Mar 31 2022, 7:55 AM
services/identity/build.rs
1 ↗(On Diff #10849)

The following line is doing a lot. compile_protos returns a Result enum, which has two variants, Ok(r) and Err(e). If compilation is successful, the function returns Ok(r) and the ? unwraps the enum. If it fails, the function returns Err(e), and the ? calls into (a trait method) to convert the error to Box<dyn std::error::Error>. The reason we Box the error here is to preserve the original error.

services/identity/src/main.rs
26 ↗(On Diff #10849)

Yeah, I'll change it to 0.0.0.0:50054.

parse() converts the &str to a SocketAddr. The rust compiler is pretty smart... it sees that addr is being used as an argument for serve() below, and that function takes a SocketAddr, so it knows to convert addr to that type when parse() is called.

32 ↗(On Diff #10849)

I don't understand this question, can you rephrase?

change ipv6 address and port

Update service implementation stub with proto changes

This revision is now accepted and ready to land.Apr 4 2022, 1:56 PM
services/identity/src/main.rs
32 ↗(On Diff #10849)

Yeah, await is a special keyword. It's not actually a method, it's just how async Rust works. See https://rust-lang.github.io/async-book/01_getting_started/04_async_await_primer.html.

services/identity/build.rs
1 ↗(On Diff #10849)

I think I get it, thanks.

services/identity/src/main.rs
26 ↗(On Diff #10849)

It, in fact, looks smart but honestly, a little confusing. Could you maybe explain it more or point out where I can look for answers (docs are ok, but the best I can get is a tutorial with a brief explanation that gets straight to the point)?

My basic question is: what is the rule here? So the compiler sees that addr has an implicit type, then it sees the parse method. At this point, I'm confused about the parse method. What is it? Is it some kind of magic method that is supposed to be implemented in certain types, or is it a global method for parsing between types?

32 ↗(On Diff #10849)

Thx Jim. Varun, I was confused about why there is .await instead of .await().

services/identity/src/main.rs
26 ↗(On Diff #10849)

https://doc.rust-lang.org/std/primitive.str.html#method.parse

parse() is a method that the str type implements. the compiler checks if SocketAddr (the implicit type of addr) implements the FromStr trait, which it does: https://doc.rust-lang.org/std/net/enum.SocketAddr.html#impl-FromStr. so, when parse() is called, SocketAddr's from_str() method is implicitly used.

converting to arch review diff

varun planned changes to this revision.Apr 6 2022, 9:39 PM

messed this up a little. needs to be changed anyway to match proto changes

update to match proto changes

This revision is now accepted and ready to land.Apr 7 2022, 12:51 PM

change socket address string to a const, revert port to 50051 since docker-compose handles the port mapping

Improve the file structure of the project