Page MenuHomePhabricator

[services] Identity - Fix proto file location
ClosedPublic

Authored by karol on May 31 2022, 12:58 AM.
Tags
None
Referenced Files
F3383087: D4156.id13219.diff
Thu, Nov 28, 1:30 PM
F3382821: D4156.diff
Thu, Nov 28, 12:07 PM
Unknown Object (File)
Tue, Nov 19, 7:15 PM
Unknown Object (File)
Fri, Nov 15, 11:34 PM
Unknown Object (File)
Sun, Nov 10, 7:35 AM
Unknown Object (File)
Sat, Nov 9, 11:41 AM
Unknown Object (File)
Sat, Nov 9, 11:41 AM
Unknown Object (File)
Fri, Nov 8, 8:08 AM

Details

Summary

I think we should have all proto files in one place. Therefore, here I'm moving the proto for identity service to native where all protos are. We can either do that or move all protos to another location but I'd have them in one place.
Additionally, I changed the folder name on the docker image from proto to protos. I this plural is better here because we may have multiple protos in each service (in the case of interservice communication, we already have that in the backup service), plus it's like this in other services so we should probably stay consistent about this.

Test Plan

Not sure how Varun has been testing the identity so far but this should work:

cd services/identity
docker-compose up --build identity-server

For now it kind of hangs, but I added explicitly:

println!("Hello");
return Ok(());

in the beginning of the main function in the main.rs file and it prints Hello. That means it successfully copies the proto file, then builds from this proto file and then compiles the sources, so I consider it working.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

karol edited the test plan for this revision. (Show Details)
karol added reviewers: varun, jim.

Agree with keeping all the protos in one place. I think this will be a good thing for when we eventually need to call the identity service from native.

That said it's probably kind of weird that we keep all of the protos in native specifically... I think it would make more sense for them to be in services somewhere, or maybe in a separate top-level directory altogether. (This can definitely be a separate diff.)

Eventually we'll probably want to access some of the services from web as well, at which point native will make less sense.

This is fine, but I also think that long term the protos should live in the services directory instead of native

This revision is now accepted and ready to land.May 31 2022, 12:01 PM