Page MenuHomePhabricator

[Keyserver/rust] Used shared identity client
ClosedPublic

Authored by varun on Aug 15 2023, 10:23 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 18, 10:54 AM
Unknown Object (File)
Sun, Nov 10, 3:59 PM
Unknown Object (File)
Sun, Nov 10, 3:59 PM
Unknown Object (File)
Sun, Nov 10, 3:59 PM
Unknown Object (File)
Sun, Nov 10, 3:59 PM
Unknown Object (File)
Sun, Nov 10, 3:59 PM
Unknown Object (File)
Sun, Nov 10, 3:59 PM
Unknown Object (File)
Sun, Nov 10, 3:58 PM
Subscribers

Details

Summary

Apply shared identity client back to rust-node-addon to
verify that it works. Also cleans up the code quite a bit.

Related to: https://linear.app/comm/issue/ENG-4640

Depends on D8804

Test Plan
  • Run identity service and localstack
  • Verify that keyserver can login to identity service upon start

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Aug 15 2023, 10:33 AM
Harbormaster failed remote builds in B21764: Diff 29917!
Harbormaster returned this revision to the author for changes because remote builds failed.Aug 15 2023, 8:27 PM
Harbormaster failed remote builds in B21775: Diff 29929!
Harbormaster returned this revision to the author for changes because remote builds failed.Aug 16 2023, 8:14 AM
Harbormaster failed remote builds in B21800: Diff 29964!

Fix docker build, not sure why previous change wasn't applied

keyserver/Dockerfile
136 ↗(On Diff #29978)

Is this really necessary? I'm afraid it will hurt performance of the Docker build here. Do we really need everything in shared/, or can we copy over what we need piecemeal?

This revision is now accepted and ready to land.Aug 16 2023, 1:54 PM
keyserver/Dockerfile
136 ↗(On Diff #29978)

Also please try to keep things consistent in this file. All other COPY commands omit the trailing slash on the first parameter

jon marked 2 inline comments as done.

Address feedback on docker COPY command

keyserver/Dockerfile
136 ↗(On Diff #29978)

We need all but cmake and tunnelbroker_messages, and the size of the folders is pretty small on a clean checkout:

$ du -hd0 ./*
4.0K	./cmake
72K	./comm-opaque2
68K	./grpc_clients
28K	./protos
28K	./tunnelbroker_messages

I just added another COPY command to keep the preference toward copying as little as needed.

136 ↗(On Diff #29978)

I assume this work is necessary for the keyserver to be able to connect to the identity service. @varun, is that right? If so, I think you'll need to commandeer it.

varun edited reviewers, added: jon; removed: varun.
This revision now requires review to proceed.Sep 11 2023, 8:33 AM
This revision is now accepted and ready to land.Sep 12 2023, 12:14 AM
This revision was automatically updated to reflect the committed changes.