Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to handle HttpClient options for external routing group selector #580

Merged
merged 3 commits into from
Jan 23, 2025

Conversation

Chaho12
Copy link
Member

@Chaho12 Chaho12 commented Dec 31, 2024

Description

Adds config options for users to change timeouts when making external routing requests.

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

* Add support to handle HttpClient options for external routing group selector. ([#579](https://github.com/trinodb/trino-gateway/pull/580))

@cla-bot cla-bot bot added the cla-signed label Dec 31, 2024
Copy link
Contributor

@willmostly willmostly left a comment

Choose a reason for hiding this comment

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

This explicit configuration can be avoided by injecting the airlift HttpClient. See ProxyRequestHandler as an example. This approach will allow configuration of any supported HttpClient options and be consistent with the proxy and monitor configs

@Chaho12 Chaho12 force-pushed the feature/jyoo/add-request-config branch 3 times, most recently from 54248cc to 158c388 Compare January 9, 2025 10:36
@Chaho12 Chaho12 requested a review from willmostly January 9, 2025 10:37
@Chaho12
Copy link
Member Author

Chaho12 commented Jan 9, 2025

@willmostly @oneonestar PTAL. I added Router config that can be used to set Airlift HttpClient configs.

@Chaho12 Chaho12 requested a review from oneonestar January 9, 2025 10:39
@Chaho12 Chaho12 changed the title Add timeout handling for external routing group selector Add support to handle HttpClient options for external routing group selector Jan 9, 2025
Copy link
Contributor

@willmostly willmostly left a comment

Choose a reason for hiding this comment

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

Looks good, just a couple minor comments from me

@Chaho12 Chaho12 force-pushed the feature/jyoo/add-request-config branch from 158c388 to 5d3491c Compare January 10, 2025 00:37
@Chaho12 Chaho12 requested a review from willmostly January 21, 2025 04:49
Copy link
Contributor

@willmostly willmostly left a comment

Choose a reason for hiding this comment

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

LGTM!

@Chaho12 Chaho12 force-pushed the feature/jyoo/add-request-config branch from 5d3491c to 46f4ff0 Compare January 22, 2025 01:09
@Chaho12 Chaho12 requested a review from ebyhr January 22, 2025 01:10
@Chaho12 Chaho12 force-pushed the feature/jyoo/add-request-config branch from 46f4ff0 to 23a22f6 Compare January 22, 2025 01:21
@Chaho12 Chaho12 requested a review from ebyhr January 22, 2025 01:22
docs/routing-rules.md Outdated Show resolved Hide resolved
docs/routing-rules.md Outdated Show resolved Hide resolved
@Chaho12 Chaho12 force-pushed the feature/jyoo/add-request-config branch from 23a22f6 to cca34bb Compare January 22, 2025 05:00
@Chaho12 Chaho12 requested a review from ebyhr January 22, 2025 05:00
docs/routing-rules.md Outdated Show resolved Hide resolved
docs/routing-rules.md Outdated Show resolved Hide resolved
@Chaho12 Chaho12 force-pushed the feature/jyoo/add-request-config branch from cca34bb to 0a27f9d Compare January 23, 2025 00:49
@Chaho12 Chaho12 requested a review from ebyhr January 23, 2025 00:49
@ebyhr ebyhr merged commit 3781459 into trinodb:main Jan 23, 2025
2 checks passed
@github-actions github-actions bot added this to the 14 milestone Jan 23, 2025
@Chaho12 Chaho12 deleted the feature/jyoo/add-request-config branch February 3, 2025 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants