Page MenuHomePhabricator

[identity] introduce new ping RPC and code version interceptor function
ClosedPublic

Authored by varun on Oct 12 2023, 10:33 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Oct 26, 10:34 PM
Unknown Object (File)
Sat, Oct 26, 10:34 PM
Unknown Object (File)
Sat, Oct 26, 10:34 PM
Unknown Object (File)
Sat, Oct 26, 10:30 PM
Unknown Object (File)
Sep 26 2024, 5:24 PM
Unknown Object (File)
Sep 19 2024, 7:31 PM
Unknown Object (File)
Sep 19 2024, 5:58 PM
Unknown Object (File)
Sep 16 2024, 4:21 AM
Subscribers

Details

Summary

pretty straightforward... just a no-op RPC that will return an unsupported version status code if the caller's code version is outdated. will set the actual unsupported versions for each platform when we launch.

Test Plan

called the RPC from kreya with metadata to trigger both an OK response and an unsupported version response

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Accepting, but please extract that version constant.

services/identity/src/grpc_services/shared.rs
5 ↗(On Diff #31985)

Minor thing but what about calling it version_interceptor (verb -> noun)?
In our codebase it acts as an "object" passed as a kind of middleware, rather than function that is called directly.

10 ↗(On Diff #31985)
const MIN_SUPPORTED_NATIVE_VERSION: u64 = 270;

we should definitely extract this as a constant.

services/identity/src/main.rs
79 ↗(On Diff #31985)

I'd call it this way, doesn't matter tho.

shared/protos/identity_client.proto
58–60 ↗(On Diff #31985)

I assume this is deleted because it's implemented in the "authenticated service", am I right?

This revision is now accepted and ready to land.Oct 16 2023, 12:55 AM
services/identity/src/grpc_services/shared.rs
5 ↗(On Diff #31985)

yeah interceptor makes more sense for sure. i was just copying the pattern introduced with the auth_intercept function. i'll change both to interceptor

10 ↗(On Diff #31985)

yeah makes sense

shared/protos/identity_client.proto
58–60 ↗(On Diff #31985)

that's right