expose crt connection timeout #5835
Conversation
|
| @@ -140,7 +140,7 @@ public final class SdkHttpConfigurationOption<T> extends AttributeMap.Key<T> { | |||
| private static final Duration DEFAULT_SOCKET_READ_TIMEOUT = Duration.ofSeconds(30); | |||
| private static final Duration DEFAULT_SOCKET_WRITE_TIMEOUT = Duration.ofSeconds(30); | |||
| private static final Duration DEFAULT_CONNECTION_TIMEOUT = Duration.ofSeconds(2); | |||
| private static final Duration DEFAULT_CONNECTION_ACQUIRE_TIMEOUT = Duration.ofSeconds(10); | |||
| private static final Duration DEFAULT_CONNECTION_ACQUIRE_TIMEOUT = Duration.ofSeconds(50); | |||
There was a problem hiding this comment.
What's the reason for this default value change?
There was a problem hiding this comment.
default value as 10 will fail the test in code build, but will succeed in my local machine. The time it takes for this test is around 30 seconds in code build, and I'm trying to figure out why that takes such a long time. This is just a test commit to check if increasing the default value will pass the test, and it passed. I'm trying to figure out a way to pass the test without changing the default value.
| @@ -126,7 +128,8 @@ private HttpClientConnectionManager createConnectionPool(URI uri) { | |||
| .withManualWindowManagement(true) | |||
| .withProxyOptions(proxyOptions) | |||
| .withMonitoringOptions(monitoringOptions) | |||
| .withMaxConnectionIdleInMilliseconds(maxConnectionIdleInMilliseconds); | |||
| .withMaxConnectionIdleInMilliseconds(maxConnectionIdleInMilliseconds) | |||
There was a problem hiding this comment.
Can we add some tests?
I know it's a bit hard to write a reliable test to verify the configuration itself, but maybe we can add tests to make sure we actually pass this to CRT?
| @@ -100,6 +100,12 @@ public BuilderT connectionTimeout(Duration connectionTimeout) { | |||
| return thisBuilder(); | |||
| } | |||
|
|
|||
| public BuilderT connectionAcquisitionTimeout(Duration connectionAcquisitionTimeout) { | |||
| Validate.isPositive(connectionAcquisitionTimeout, "connectionAcquisitionTimeout"); | |||
Motivation and Context
CRT HTTP client Java binding added support for connection acquire timeout, we also need to expose the connectionAcquisitionTimeout field in crt http client and pass it through
Modifications
Testing
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License