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.

Sign up for GitHub

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

Jump to bottom

Added support for setting the read timeout in http config #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added support for setting the read timeout in http config #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 27 additions & 25 deletions HorizonApache/src/main/java/com/hubspot/horizon/apache/ApacheHttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,30 @@
package com.hubspot.horizon.apache;

import java.io.IOException;
import java.io.InputStream;
import java.util.Timer;
import java.util.TimerTask;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean;

import javax.annotation.Nullable;

import org.apache.http.HttpEntity;
import org.apache.http.client.ClientProtocolException;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.config.Registry;
import org.apache.http.config.RegistryBuilder;
import org.apache.http.config.SocketConfig;
import org.apache.http.conn.HttpClientConnectionManager;
import org.apache.http.conn.socket.ConnectionSocketFactory;
import org.apache.http.conn.socket.PlainConnectionSocketFactory;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.base.Preconditions;
import com.google.common.base.Throwables;
import com.hubspot.horizon.HttpClient;
Expand All @@ -18,29 +43,6 @@
import com.hubspot.horizon.apache.internal.KeepAliveWithDefaultStrategy;
import com.hubspot.horizon.apache.internal.LenientRedirectStrategy;
import com.hubspot.horizon.apache.internal.SnappyContentEncodingResponseInterceptor;
import org.apache.http.HttpEntity;
import org.apache.http.client.ClientProtocolException;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.config.Registry;
import org.apache.http.config.RegistryBuilder;
import org.apache.http.config.SocketConfig;
import org.apache.http.conn.HttpClientConnectionManager;
import org.apache.http.conn.socket.ConnectionSocketFactory;
import org.apache.http.conn.socket.PlainConnectionSocketFactory;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.Nullable;
import java.io.IOException;
import java.io.InputStream;
import java.util.Timer;
import java.util.TimerTask;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean;

public class ApacheHttpClient implements HttpClient {
private static final Logger LOG = LoggerFactory.getLogger(ApacheHttpClient.class);
Expand Down Expand Up @@ -98,15 +100,15 @@ private RequestConfig createRequestConfig(HttpConfig config) {
return RequestConfig.custom()
.setConnectionRequestTimeout(config.getConnectTimeoutMillis())
.setConnectTimeout(config.getConnectTimeoutMillis())
.setSocketTimeout(config.getRequestTimeoutMillis())
.setSocketTimeout(config.getReadTimeoutMillis())
Copy link
Member

Choose a reason for hiding this comment

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

Won't this break anyone who was setting requestTimeoutMillis?

.setRedirectsEnabled(config.isFollowRedirects())
.setMaxRedirects(config.getMaxRedirects())
.setRelativeRedirectsAllowed(config.isRejectRelativeRedirects())
.build();
}

private SocketConfig createSocketConfig(HttpConfig config) {
return SocketConfig.custom().setSoTimeout(config.getRequestTimeoutMillis()).build();
return SocketConfig.custom().setSoTimeout(config.getReadTimeoutMillis()).build();
}

@Override
Expand Down
18 changes: 16 additions & 2 deletions HorizonCore/src/main/java/com/hubspot/horizon/HttpConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
package com.hubspot.horizon;

import java.util.concurrent.TimeUnit;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.base.Preconditions;
import com.google.common.primitives.Ints;
import com.hubspot.horizon.HttpRequest.Options;

import java.util.concurrent.TimeUnit;

public class HttpConfig {
private final int maxConnections;
private final int maxConnectionsPerHost;
private final int connectTimeoutSeconds;
private final int readTimeoutSeconds;
private final int requestTimeoutSeconds;
private final int defaultKeepAliveSeconds;
private final int maxRedirects;
Expand All @@ -28,6 +29,7 @@ private HttpConfig(int maxConnections,
int maxConnectionsPerHost,
int connectTimeoutSeconds,
int requestTimeoutSeconds,
int readTimeoutSeconds,
int defaultKeepAliveSeconds,
int maxRedirects,
String userAgent,
Expand All @@ -43,6 +45,7 @@ private HttpConfig(int maxConnections,
this.maxConnectionsPerHost = maxConnectionsPerHost;
this.connectTimeoutSeconds = connectTimeoutSeconds;
this.requestTimeoutSeconds = requestTimeoutSeconds;
this.readTimeoutSeconds = readTimeoutSeconds;
this.defaultKeepAliveSeconds = defaultKeepAliveSeconds;
this.maxRedirects = maxRedirects;
this.userAgent = userAgent;
Expand Down Expand Up @@ -76,6 +79,10 @@ public int getRequestTimeoutMillis() {
return Ints.checkedCast(TimeUnit.SECONDS.toMillis(requestTimeoutSeconds));
}

public int getReadTimeoutMillis() {
return Ints.checkedCast(TimeUnit.SECONDS.toMillis(readTimeoutSeconds));
}

public int getDefaultKeepAliveMillis() {
return Ints.checkedCast(TimeUnit.SECONDS.toMillis(defaultKeepAliveSeconds));
}
Expand Down Expand Up @@ -120,6 +127,7 @@ public static class Builder {
private int maxConnectionsPerHost = 25;
private int connectTimeoutSeconds = 1;
private int requestTimeoutSeconds = 30;
private int readTimeoutSeconds = 30;
private int defaultKeepAliveSeconds = 10;
private int maxRedirects = 10;
private String userAgent = "Horizon/0.0.1";
Expand Down Expand Up @@ -154,6 +162,11 @@ public Builder setRequestTimeoutSeconds(int requestTimeoutSeconds) {
return this;
}

public Builder setReadTimeoutSeconds(int readTimeoutSeconds) {
this.readTimeoutSeconds = readTimeoutSeconds;
return this;
}

public Builder setDefaultKeepAliveSeconds(int defaultKeepAliveSeconds) {
this.defaultKeepAliveSeconds = defaultKeepAliveSeconds;
return this;
Expand Down Expand Up @@ -214,6 +227,7 @@ public HttpConfig build() {
maxConnectionsPerHost,
connectTimeoutSeconds,
requestTimeoutSeconds,
readTimeoutSeconds,
defaultKeepAliveSeconds,
maxRedirects,
userAgent,
Expand Down
3 changes: 1 addition & 2 deletions HorizonNing/src/main/java/com/hubspot/horizon/ning/NingAsyncHttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import com.hubspot.horizon.ning.internal.NingRetryHandler;
import com.hubspot.horizon.ning.internal.NingSSLContext;
import com.ning.http.client.AsyncHttpClientConfig;
import com.ning.http.client.AsyncHttpProviderConfig;
import com.ning.http.client.Request;
import com.ning.http.client.extra.ThrottleRequestFilter;
import com.ning.http.client.providers.netty.NettyAsyncHttpProviderConfig;
Expand Down Expand Up @@ -77,7 +76,7 @@ public NingAsyncHttpClient(HttpConfig config) {
.setMaxConnectionsPerHost(config.getMaxConnectionsPerHost())
.setConnectTimeout(config.getConnectTimeoutMillis())
.setRequestTimeout(config.getRequestTimeoutMillis())
.setReadTimeout(config.getRequestTimeoutMillis())
.setReadTimeout(config.getReadTimeoutMillis())
.setMaxRedirects(config.getMaxRedirects())
.setFollowRedirect(config.isFollowRedirects())
.setHostnameVerifier(new NingHostnameVerifier(config.getSSLConfig()))
Expand Down