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

Upgrade async http client #345

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
v5.2.0
------
* Upgrade AsyncHttpClient - Backwards incompatible due to namespace of AsyncHttpClient changing along with class names and private vs public access.

v5.1.17
------
* Use dedicated executor service for lambda analysis
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
version=5.1.17
version=5.2.0
group=com.linkedin.parseq
org.gradle.parallel=true
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import com.linkedin.parseq.example.common.AbstractExample;
import com.linkedin.parseq.example.common.ExampleUtil;
import com.linkedin.parseq.httpclient.HttpClient;
import com.ning.http.client.Response;
import org.asynchttpclient.Response;


/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import com.linkedin.parseq.function.Success;
import com.linkedin.parseq.function.Try;
import com.linkedin.parseq.httpclient.HttpClient;
import com.ning.http.client.Response;


/**
Expand Down
2 changes: 1 addition & 1 deletion subprojects/parseq-http-client/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ ext {
}

dependencies {
compile group: 'com.ning', name: 'async-http-client', version:'1.9.21'
api group: 'org.asynchttpclient', name: 'async-http-client', version:'2.12.3'
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package com.linkedin.parseq.httpclient;

import java.io.IOException;
import java.util.concurrent.atomic.AtomicReference;

import com.ning.http.client.AsyncHttpClient;
import com.ning.http.client.AsyncHttpClientConfig;
import org.asynchttpclient.AsyncHttpClient;
import org.asynchttpclient.AsyncHttpClientConfig;
import org.asynchttpclient.DefaultAsyncHttpClientConfig.Builder;
import org.asynchttpclient.Dsl;


public class HttpClient {

Expand All @@ -15,9 +19,9 @@ public class HttpClient {
* then new client is created with default configuration.
* @return raw http client
*/
public static synchronized AsyncHttpClient getNingClient() {
public static synchronized AsyncHttpClient getAsyncHttpClient() {
if (_client.get() == null) {
initialize(new AsyncHttpClientConfig.Builder().build());
initialize(new Builder().build());
}
return _client.get();
}
Comment on lines +22 to 27
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should instead introduce getAsyncHttpClient() alongside getNingClient() and mark getNingClient() as @Deprecated. I know the type has changed, and there's nothing we can do about that, but there are users of this code that will break if getNingClient() disappears on a minor version bump.

Copy link
Contributor

Choose a reason for hiding this comment

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

I need to confer with the team about the other breaking changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only concern I would have with that is if ning needs a different version of netty (or other) that the new project needs. What are your thoughts on that behavior? Would you like to see the dependency resolution for netty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Expand All @@ -29,43 +33,43 @@ public static synchronized AsyncHttpClient getNingClient() {
*/
@SuppressWarnings("resource")
public static synchronized void initialize(AsyncHttpClientConfig cfg) {
if (!_client.compareAndSet(null, new AsyncHttpClient(cfg))) {
if (!_client.compareAndSet(null, Dsl.asyncHttpClient(cfg))) {
throw new RuntimeException("async http client concurrently initialized");
}
}

public static void close() {
public static void close() throws IOException {
if (_client.get() != null) {
_client.get().close();
}
}

public static WrappedRequestBuilder get(String url) {
return new WrappedRequestBuilder(getNingClient().prepareGet(url), "GET " + url);
return new WrappedRequestBuilder(getAsyncHttpClient().prepareGet(url), "GET " + url);
}

public static WrappedRequestBuilder connect(String url) {
return new WrappedRequestBuilder(getNingClient().prepareConnect(url), "CONNECT " + url);
return new WrappedRequestBuilder(getAsyncHttpClient().prepareConnect(url), "CONNECT " + url);
}

public static WrappedRequestBuilder options(String url) {
return new WrappedRequestBuilder(getNingClient().prepareOptions(url), "OPTIONS " + url);
return new WrappedRequestBuilder(getAsyncHttpClient().prepareOptions(url), "OPTIONS " + url);
}

public static WrappedRequestBuilder head(String url) {
return new WrappedRequestBuilder(getNingClient().prepareHead(url), "HEAD " + url);
return new WrappedRequestBuilder(getAsyncHttpClient().prepareHead(url), "HEAD " + url);
}

public static WrappedRequestBuilder post(String url) {
return new WrappedRequestBuilder(getNingClient().preparePost(url), "POST " + url);
return new WrappedRequestBuilder(getAsyncHttpClient().preparePost(url), "POST " + url);
}

public static WrappedRequestBuilder put(String url) {
return new WrappedRequestBuilder(getNingClient().preparePut(url), "PUT " + url);
return new WrappedRequestBuilder(getAsyncHttpClient().preparePut(url), "PUT " + url);
}

public static WrappedRequestBuilder delete(String url) {
return new WrappedRequestBuilder(getNingClient().prepareDelete(url), "DELETE " + url);
return new WrappedRequestBuilder(getAsyncHttpClient().prepareDelete(url), "DELETE " + url);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,21 @@
import com.linkedin.parseq.Task;
import com.linkedin.parseq.promise.Promises;
import com.linkedin.parseq.promise.SettablePromise;
import com.ning.http.client.AsyncHttpClient.BoundRequestBuilder;
import com.ning.http.client.AsyncCompletionHandler;
import com.ning.http.client.BodyGenerator;
import com.ning.http.client.ConnectionPoolPartitioning;
import com.ning.http.client.FluentCaseInsensitiveStringsMap;
import com.ning.http.client.Param;
import com.ning.http.client.ProxyServer;
import com.ning.http.client.Realm;
import com.ning.http.client.Request;
import com.ning.http.client.Response;
import com.ning.http.client.SignatureCalculator;
import com.ning.http.client.cookie.Cookie;
import com.ning.http.client.multipart.Part;
import com.ning.http.client.uri.Uri;
import org.asynchttpclient.BoundRequestBuilder;

import org.asynchttpclient.AsyncCompletionHandler;
import org.asynchttpclient.request.body.generator.BodyGenerator;
import org.asynchttpclient.channel.ChannelPoolPartitioning;
import io.netty.handler.codec.http.HttpHeaders;
import org.asynchttpclient.Param;
import org.asynchttpclient.proxy.ProxyServer;
import org.asynchttpclient.Realm;
import org.asynchttpclient.Request;
import org.asynchttpclient.Response;
import org.asynchttpclient.SignatureCalculator;
import io.netty.handler.codec.http.cookie.Cookie;
import org.asynchttpclient.request.body.multipart.Part;
import org.asynchttpclient.uri.Uri;

public class WrappedRequestBuilder {

Expand Down Expand Up @@ -58,12 +59,12 @@ public WrappedRequestBuilder addBodyPart(Part part) {
}

public WrappedRequestBuilder setInetAddress(InetAddress address) {
_delegate.setInetAddress(address);
_delegate.setAddress(address);
return this;
}

public WrappedRequestBuilder setLocalInetAddress(InetAddress address) {
_delegate.setLocalInetAddress(address);
_delegate.setLocalAddress(address);
return this;
}

Expand Down Expand Up @@ -101,11 +102,6 @@ public WrappedRequestBuilder setBody(InputStream stream) {
return this;
}

public WrappedRequestBuilder setContentLength(int length) {
_delegate.setContentLength(length);
return this;
}

public WrappedRequestBuilder setBody(String data) {
_delegate.setBody(data);
return this;
Expand All @@ -121,7 +117,7 @@ public WrappedRequestBuilder setCookies(Collection<Cookie> cookies) {
return this;
}

public WrappedRequestBuilder setHeaders(FluentCaseInsensitiveStringsMap headers) {
public WrappedRequestBuilder setHeaders(HttpHeaders headers) {
_delegate.setHeaders(headers);
return this;
}
Expand Down Expand Up @@ -222,7 +218,7 @@ public WrappedRequestBuilder setRealm(Realm realm) {
}

public WrappedRequestBuilder setFollowRedirects(boolean followRedirects) {
_delegate.setFollowRedirects(followRedirects);
_delegate.setFollowRedirect(followRedirects);
return this;
}

Expand All @@ -241,13 +237,8 @@ public WrappedRequestBuilder setMethod(String method) {
return this;
}

public WrappedRequestBuilder setBodyEncoding(String charset) {
_delegate.setBodyEncoding(charset);
return this;
}

public WrappedRequestBuilder setConnectionPoolKeyStrategy(ConnectionPoolPartitioning connectionPoolKeyStrategy) {
_delegate.setConnectionPoolKeyStrategy(connectionPoolKeyStrategy);
public WrappedRequestBuilder setConnectionPoolKeyStrategy(ChannelPoolPartitioning connectionPoolKeyStrategy) {
_delegate.setChannelPoolPartitioning(connectionPoolKeyStrategy);
return this;
}

Expand All @@ -257,7 +248,7 @@ public Task<Response> task(final String desc) {
_delegate.execute(new AsyncCompletionHandler<Response>() {

@Override
public Response onCompleted(final Response response) throws Exception {
public Response onCompleted(final Response response) {
result.done(response);
return response;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.asynchttpclient.Response;
import org.codehaus.jackson.JsonNode;
import org.codehaus.jackson.map.ObjectMapper;
import org.eclipse.jetty.server.Request;
Expand All @@ -29,7 +30,7 @@
import com.linkedin.parseq.trace.TraceBuilder;
import com.linkedin.parseq.trace.TraceRelationship;
import com.linkedin.parseq.trace.codec.json.JsonTraceCodec;
import com.ning.http.client.Response;


final class JhatHandler extends AbstractHandler {

Expand Down
Loading