Add withRequestConfig to CommonsHttpClient.Builder#691
Open
renaudhartert-db wants to merge 3 commits intomainfrom
Open
Add withRequestConfig to CommonsHttpClient.Builder#691renaudhartert-db wants to merge 3 commits intomainfrom
renaudhartert-db wants to merge 3 commits intomainfrom
Conversation
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Allow users to pass a custom Apache RequestConfig for fine-grained timeout control (e.g. short connect timeout with long socket timeout). The existing withTimeoutSeconds remains for the common case. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
renaudhartert-db
force-pushed
the
renaud-hartert/add-request-config-builder
branch
from
a2d6213 to
ce927e5
Compare
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
renaudhartert-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add
withRequestConfig(RequestConfig)toCommonsHttpClient.Builderso that users can independently configure connect, socket, and connection-request timeouts on the underlying Apache HttpClient.Why
CommonsHttpClient.Builder.withTimeoutSeconds()sets all three Apache HttpClient timeouts (connect, socket, connection-request) to the same value. Users who need long socket timeouts for slow queries (e.g. 15 minutes) are forced to also set long connect timeouts, which means the application hangs for that entire duration if a host is unreachable.Rather than mirroring Apache's
RequestConfigAPI one field at a time on the builder, this PR exposes theRequestConfigdirectly.CommonsHttpClientalready exposes other Apache types (SSLConnectionSocketFactory,PoolingHttpClientConnectionManager,HttpRequestRetryHandler), so this is consistent with the existing abstraction boundary. It also means any futureRequestConfigoptions are available without further SDK changes.What changed
Interface changes
CommonsHttpClient.Builder.withRequestConfig(RequestConfig)— accepts a custom ApacheRequestConfig. When set, it takes precedence overwithTimeoutSeconds()and any timeout fromDatabricksConfig.withTimeoutSeconds()to referencewithRequestConfig()as the fine-grained alternative.Behavioral changes
None. Existing users who don't call
withRequestConfig()get the same behavior as before.Internal changes
makeDefaultRequestConfig(DatabricksConfig, Integer)static method for clarity.How is this tested?
testCustomRequestConfigtest inCommonsHttpClientTestbuilds a client with separate connect (10s), socket (900s), and connection-request (300s) timeouts and verifies it executes a request successfully.CommonsHttpClientTesttests pass.