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
Branch
jonringer/identity-client
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.