Skip to content

[Internal] Thin Client Integration: Adds HTTP Client Sharing between Gateway and Thin Client#5141

Merged
kirankumarkolli merged 8 commits intomasterfrom
users/aavasthy/4579_httpclientSharing
Apr 30, 2025
Merged

[Internal] Thin Client Integration: Adds HTTP Client Sharing between Gateway and Thin Client#5141
kirankumarkolli merged 8 commits intomasterfrom
users/aavasthy/4579_httpclientSharing

Conversation

@aavasthy
Copy link
Copy Markdown
Contributor

Pull Request Template

Description

Adding HttpClient sharing logic between Gateway and Thinclient model. HttpClient is capable of handling both Http1.1 and Http2.0 protocol. The request will decide which protocol to use based on the connection mode.

Therefore, ideally it should be possible to share the same client with both the (Gateway and Thin Client) store models.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Closing issues

To automatically close an issue: closes #4579

Copy link
Copy Markdown
Member

@FabianMeiswinkel FabianMeiswinkel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - I was initially sceptical whether sharing HttpClient between normal Gateway (possibly HTTP 1.1) and ThinClient is a good idea. But trying to poke holes in that idea I don't see any obvious problem. If customer configures HttpClient to not allow HTTP/2 seeing failures for ThinClient is expected i guess. You could mess-up the normal Gateway HttpClient if something goes wrong with ThinClient - but this is something we can properly test.

@aavasthy aavasthy added the auto-merge Enables automation to merge PRs label Apr 29, 2025
@aavasthy
Copy link
Copy Markdown
Contributor Author

LGTM - I was initially sceptical whether sharing HttpClient between normal Gateway (possibly HTTP 1.1) and ThinClient is a good idea. But trying to poke holes in that idea I don't see any obvious problem. If customer configures HttpClient to not allow HTTP/2 seeing failures for ThinClient is expected i guess. You could mess-up the normal Gateway HttpClient if something goes wrong with ThinClient - but this is something we can properly test.

Tested these changes with both gateway mode and thinclient mode by enabling and disabling thinclient flag. For regression, I am relying on existing pipeline tests we have. Is there anything else we should cover as part of testing this change?

@kirankumarkolli kirankumarkolli merged commit 0b90eee into master Apr 30, 2025
26 checks passed
@kirankumarkolli kirankumarkolli deleted the users/aavasthy/4579_httpclientSharing branch April 30, 2025 17:16
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Azure Cosmos SDKs Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge Enables automation to merge PRs thin-client-integration

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Thin Client Integration] Analyze and Implement HTTP Client Sharing between Gateway and Thin Client

4 participants