Previously, all devices were being registered as
a "client" regardless.
https://linear.app/comm/issue/ENG-4521
Depends on D8627
Details
Details
- Reviewers
bartek varun - Commits
- rCOMMc99660536068: Allow for devices to register as a keyserver
Integration tests
Register keyserver locally, and ensure it displays as a "keyserver" deviceType.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| keyserver/addons/rust-node-addon/src/identity_client/register_user.rs | ||
|---|---|---|
| 45 ↗ | (On Diff #29081) | even though tonic generates an Enum, protobuf still expects an i32 |
| services/identity/src/database.rs | ||
| 71–75 ↗ | (On Diff #29081) | I assumed we would want to differentiate between these three. But I could be wrong. |
| 77–93 ↗ | (On Diff #29081) | Not sure if there's a cleaner way, but this is what first came to mind. I don't personally like that I'm duplicating the value to enum mapping. |
| 1007 ↗ | (On Diff #29081) | This was the previous hardcoding of the value. |
| shared/protos/identity_client.proto | ||
| 108 ↗ | (On Diff #29081) | proto3 requires this to start at 0 |
Comment Actions
While we're here, should we add codeVersion?
The login/registration events should really only occur when the device needs to retrieve a user_id or commServicesAccessToken, I don't think codeVersion will reflect an up-to-date value.
Comment Actions
Rust LGTM, not much context on splitting the client device type into native and web - was this discussed anywhere?. Makes sense to me though.
Comment Actions
- We need to make sure that the identity service has the codeVersion of the client. When we launch the device list work, the identity service will need to be able to identify whether a given client is able to support the "new" workflow and function as a primary device. This is just one example of many... it's critical that Comm services are aware of the client version
- @jon mentioned that this value won't be up-to-date, but that's better than nothing. If in the future we need a more up-to-date value, we can have newer clients call some other identity service RPC when their version changes
- As discussed offline, let's break this down into 'ios' | 'android' | 'web' | 'macos' | 'windows'
| native/native_rust_library/src/lib.rs | ||
|---|---|---|
| 234 ↗ | (On Diff #29446) | not sure how to get this to determine DeviceType::iOS vs DeviceType::Android |
| native/native_rust_library/src/lib.rs | ||
|---|---|---|
| 234 ↗ | (On Diff #29446) | A compile-time flag set per platform? |
Comment Actions
We need to make sure that the identity service has the codeVersion of the client. When we launch the device list work, the identity service will need to be able to identify whether a given client is able to support the "new" workflow and function as a primary device. This is just one example of many... it's critical that Comm services are aware of the client version
I think this is out-of-scope for this diff, which is just trying to fix a shortcoming.
https://linear.app/comm/issue/ENG-4587 is tracking this issue
As discussed offline, let's break this down into 'ios' | 'android' | 'web' | 'macos' | 'windows'
Added to current goal as https://linear.app/comm/issue/ENG-4608/
| native/native_rust_library/src/lib.rs | ||
|---|---|---|
| 234 ↗ | (On Diff #29446) | Discussion: https://linear.app/comm/issue/ENG-4608 |
Comment Actions
We're now tracking the goal of making identity service aware of client codeVersion in ENG-4598