Page MenuHomePhabricator

[grpc clients] use cookie for finish requests
ClosedPublic

Authored by varun on Nov 30 2023, 3:43 PM.
Tags
None
Referenced Files
F3333716: D10121.id34197.diff
Thu, Nov 21, 5:01 AM
F3333715: D10121.id34164.diff
Thu, Nov 21, 5:01 AM
F3333714: D10121.id34163.diff
Thu, Nov 21, 5:01 AM
F3333712: D10121.id34080.diff
Thu, Nov 21, 5:01 AM
F3333709: D10121.id.diff
Thu, Nov 21, 5:01 AM
F3333707: D10121.diff
Thu, Nov 21, 5:01 AM
Unknown Object (File)
Fri, Nov 1, 1:24 AM
Unknown Object (File)
Wed, Oct 23, 11:24 AM
Subscribers

Details

Summary

when we call the registration, login, and update password _finish RPCs, we need to use the load balancer cookie to ensure these requests are routed to the same identity service instance that received the _start RPCs.

Test Plan

called the native_rust_library client methods and confirmed that registration/login/update password all worked on staging with multiple instances running

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun requested review of this revision.Nov 30 2023, 4:27 PM
bartek requested changes to this revision.Dec 1 2023, 12:36 AM

I'm glad that you've figured it out! 🎉

I have one concern inline, feel free to re-request if I've been missing something

shared/grpc_clients/src/error.rs
34–36 ↗(On Diff #34080)

I find this more universal

shared/grpc_clients/src/identity/shared.rs
63–65 ↗(On Diff #34080)

What about local dev environment? This cookie won't be available.
Should we return Result<Option<MetadataValue>> and then do if let Some(cookie) = ...?

This revision now requires changes to proceed.Dec 1 2023, 12:36 AM

addressed feedback and decided some of the error handling was unnecessary

use constant to avoid re-typing "set-cookie" and "cookie"

This revision is now accepted and ready to land.Dec 2 2023, 12:02 AM