Add ability to set and get network timeout#1185
Open
madhav-db wants to merge 6 commits intodatabricks:mainfrom
Open
Add ability to set and get network timeout#1185madhav-db wants to merge 6 commits intodatabricks:mainfrom
madhav-db wants to merge 6 commits intodatabricks:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request implements the setNetworkTimeout() and getNetworkTimeout() methods on the JDBC Connection interface, addressing issue #1140. The implementation allows users to configure network I/O timeout for database operations.
Changes:
- Added network timeout storage in
DatabricksConnectionContextwith getter/setter methods - Implemented
setNetworkTimeout()andgetNetworkTimeout()inDatabricksConnectionwith input validation - Modified
DatabricksHttpClientto use network timeout when set, falling back to socket timeout otherwise
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/databricks/jdbc/api/impl/DatabricksConnection.java | Implements setNetworkTimeout/getNetworkTimeout methods with validation for negative values |
| src/main/java/com/databricks/jdbc/api/internal/IDatabricksConnectionContext.java | Adds interface methods for getting and setting network timeout |
| src/main/java/com/databricks/jdbc/api/impl/DatabricksConnectionContext.java | Stores network timeout as volatile int field with getter/setter implementation |
| src/main/java/com/databricks/jdbc/dbclient/impl/http/DatabricksHttpClient.java | Updates RequestConfig creation to prefer network timeout over socket timeout |
| src/test/java/com/databricks/jdbc/api/impl/DatabricksConnectionTest.java | Adds basic tests for network timeout getter/setter and negative value validation |
| NEXT_CHANGELOG.md | Documents the new feature in changelog |
| .metals/metals.lock.db | IDE-generated lock file (should not be committed) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/test/java/com/databricks/jdbc/api/impl/DatabricksConnectionTest.java
Show resolved
Hide resolved
src/main/java/com/databricks/jdbc/dbclient/impl/http/DatabricksHttpClient.java
Outdated
Show resolved
Hide resolved
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Address Copilot feedback: Network timeout now takes effect immediately by applying RequestConfig per-request instead of only during HTTP client initialization. This allows setNetworkTimeout() to work correctly without requiring HTTP client recreation. Changes: - Apply RequestConfig dynamically in execute() when network timeout is set - Import HttpRequestBase to enable per-request config override - Update CHANGELOG to clarify dynamic timeout application Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
gopalldb
reviewed
Jan 22, 2026
| if (connectionContext != null) { | ||
| int networkTimeout = connectionContext.getNetworkTimeout(); | ||
| if (networkTimeout > 0 && request instanceof HttpRequestBase) { | ||
| RequestConfig requestConfig = makeRequestConfig(connectionContext); |
Collaborator
There was a problem hiding this comment.
what if there is already a request config? You would be overwriting that.
gopalldb
reviewed
Jan 22, 2026
| if (connectionContext != null) { | ||
| int networkTimeout = connectionContext.getNetworkTimeout(); | ||
| if (networkTimeout > 0 && request instanceof HttpRequestBase) { | ||
| RequestConfig requestConfig = makeRequestConfig(connectionContext); |
Collaborator
There was a problem hiding this comment.
This will only affect Cloud fetch and Thrift server client. This won't affect SDK Http client.
gopalldb
reviewed
Jan 22, 2026
| int networkTimeout = connectionContext.getNetworkTimeout(); | ||
| if (networkTimeout > 0 && request instanceof HttpRequestBase) { | ||
| RequestConfig requestConfig = makeRequestConfig(connectionContext); | ||
| ((HttpRequestBase) request).setConfig(requestConfig); |
Collaborator
There was a problem hiding this comment.
From the JDBC spec, the network timeout seems to consider across all retried requests, whereas you are setting it for each request. I will increase timeout for overall user request end to end
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
2 participants
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.
Description
Add ability to set and get network timeout
Addresses: #1140
Testing
Additional Notes to the Reviewer