[Internal]Thin Client Integration: Adds Classes and Methods for Transport Serialization of RNTBD Payload #5019
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
aavasthy
changed the title
aavasthy
requested review from
khdang,
sboshra,
adityasa,
neildsh,
kirankumarkolli,
FabianMeiswinkel,
kirillg and
Pilchie
as code owners
aavasthy
changed the title
| out _); | ||
|
|
||
| // TODO: consider using the SerializedRequest directly. | ||
| MemoryStream memoryStream = new MemoryStream(serializedRequest.RequestSize); |
There was a problem hiding this comment.
This is pretty inefficient - why not change the method signature of BuildRequestForProxy to return a stream instead?
There was a problem hiding this comment.
Also why would we want to push any ThinClient specific logic into SharedFiles - why not use the Classes in SharedFile to create the rntbd envelope from V3 repo? We might need to iterate on this quickly?
| this.Dispose(true); | ||
| } | ||
|
|
||
| private async Task CaptureSessionTokenAndHandleSplitAsync( |
There was a problem hiding this comment.
GatewayStoreModel also has an implementation for CaptureSessionTokenAndHandleSplitAsync. See these lines of code for details.
Can we consolidate the code in one common place, so that is parts of it are common, then we need to make sure there are no code duplications.
| } | ||
| } | ||
|
|
||
| internal static async Task ApplySessionTokenAsync( |
There was a problem hiding this comment.
Ditto. Code duplication. Let's consolidate in one common place if they are similar.
| @@ -38,5 +38,6 @@ internal interface IGlobalEndpointManager : IDisposable | |||
| ReadOnlyDictionary<string, Uri> GetAvailableReadEndpointsByLocation(); | |||
|
|
|||
| bool CanSupportMultipleWriteLocations(ResourceType resourceType, OperationType operationType); | |||
| bool TryGetLocationForGatewayDiagnostics(Uri endpoint, out string regionName); | |||
There was a problem hiding this comment.
Do we need this method exposed in the interface ? I don't see it's used in any of the tests. If not - Let's get this removed.
Pull Request Template
Description
This pull request introduces the ProxyStoreClient, ThinClientStoreModel, and ThinClientTransportSerializer classes as part of SDK-Thinclient integration. These additions enhance the SDK's functionality by enabling operations through a ThinClient proxy, which includes applying session tokens, resolving partition key ranges, and delegating requests to the ProxyStoreClient. The ThinClientTransportSerializer provides serialization and deserialization of requests and responses to and from the RNTBD protocol for the ThinClient scenario.
Also, included functional unit tests for these classes.
ThinClientStoreModel: This class implements the
ProcessMessageAsync()method from theIStoreModelinterface, to send the request message via the proxy store client. This store model abstracts out the implementation of the thin client interaction using the new Http 2 protocol.ProxyStoreClient: This client is responsible for interacting with the thin client over Http2, using the thin client uri.
ThinClientTransportSerializer: This class provides static methods for serializing and deserializing requests and responses to and from the RNTBD protocol for the ThinClient scenario.
Type of change
Please delete options that are not relevant.
Closing issues
To automatically close an issue: closes #4571