[keyserver] reorganize rust-node-addon
ClosedPublic
Actions

Authored by varun on Feb 28 2023, 1:15 PM.

Details

Reviewers

• jon
bartek
tomek

Commits

rCOMM4a38858145ac: [keyserver] reorganize rust-node-addon

Summary

before I put up the diffs for login_user, I wanted to do a little bit of reorganizing. now, delete_user and register_user are submodules of the identity_client module, which is more intuitive IMO.

I also added an interceptor in the delete_user fn so that we can authorize delete requests to the Identity service

There is some repeated code, but I didn't want to spend more time figuring out how to de-duplicate it. Updated the following task to track this work: https://linear.app/comm/issue/ENG-2733/refactor-native-rust-library-and-rust-node-addon-client-logic

Test Plan

tested the DeleteUser RPC with the new interceptor, things worked as expected

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

varun created this revision.Feb 28 2023, 1:15 PM

Herald added subscribers: atul, ashoat. · View Herald TranscriptFeb 28 2023, 1:15 PM

Harbormaster completed remote builds in B17011: Diff 23265.Feb 28 2023, 1:34 PM

varun requested review of this revision.Feb 28 2023, 1:34 PM

mostly looks good.

keyserver/addons/rust-node-addon/src/identity_client/mod.rs
75 ↗	(On Diff #23265)	It would probably be worthwhile to have the pake factored out to it's own module. But we can tackle that in https://linear.app/comm/issue/ENG-3147

This revision is now accepted and ready to land.Feb 28 2023, 1:56 PM

I think it would've been good to separate this into two diffs: one that moves the code around, and the other than changes things (eg. the interceptor that you added). More details here

In D6913#205664, @ashoat wrote:

I think it would've been good to separate this into two diffs: one that moves the code around, and the other than changes things (eg. the interceptor that you added). More details here

yeah fair, i'll make sure to do that next time

Closed by commit rCOMM4a38858145ac: [keyserver] reorganize rust-node-addon (authored by varun). · Explain WhyMar 2 2023, 6:41 AM

This revision was automatically updated to reflect the committed changes.

varun added a commit: rCOMM4a38858145ac: [keyserver] reorganize rust-node-addon.