Upgrade Async-http-client


(Fatih) #1

Hi,
I appreciate the efforts are more for Play 3 these days which may kill using AsyncHttpClient and promote to use Akka Http client, but for the existing apps we have the following requirement.

We wanted to upgrade async-http-client used in play-ws to be able to use the new connection usage metrics exposed in the later version of the dependency.

We have couple of points that we weren’t sure what’s the best way to fix.

  • Can we use STRICT CookieDecoder by default where we use cookieDecoder before?
  • Can you verify the change in StandaloneAhcWSRequest regarding to Source<> usage?

We also NF tested those changes under 1000tps inbound and 2000tps outbound calls.
Happy to create PR after clarifying those steps.


(Rich Dougherty) #2

Hi Fatih

We wanted to upgrade async-http-client used in play-ws to be able to use the new connection usage metrics exposed in the later version of the dependency.

For existing users of Play WS are there any compatibility concerns? I don’t have any issue with upgrading so long as it doesn’t cause problems for users. Based on the number of changes to AHC between 2.0 and 2.4 we will probably need to do this in a major release of Play WS, just to be safe.

I will also double-check this with @marcospereira who is away at the moment but will be back at the end of next week.

not able to put more than 2 links

You should be able to do that now. :slight_smile:

Can we use STRICT CookieDecoder by default where we use cookieDecoder before?

So the choice is between STRICT and LAX? (https://netty.io/4.1/api/io/netty/handler/codec/http/cookie/ClientCookieDecoder.html) I suppose it could be configurable?

Those changes look like they will work. You could make them more efficient by using wrappedBuffer instead of copiedBuffer, since ByteString.toByteBuffer already makes a copy, I believe.


(Fatih) #3

Hi Rich,

Amended first commit to apply your second suggestion.
And we have another commit for making cookie decoder configurable.

Shall we create a PR after those changes if you don’t have more points to improve?

Cheers
Fatih & Deepak


(Rich Dougherty) #4

Hi Fatih, yes please create a PR and ping me here when it’s available. As I mentioned earlier I’ll run this past @marcospereira when he gets back.


(Fatih) #5

There you have https://github.com/playframework/play-ws/pull/252


(Marcos Pereira) #6

Thank you, @fatihi.

We usually don’t do these major library updates for stable versions of Play (updating AsyncHttpClient will eventually cascade from Play-WS to Play itself). Play 2.6.x is in a state we prefer stability and compatibility as much as possible.

But we can have some middle ground here.

Maybe release a version 1.2.x of play-ws that you can update by yourself on your application.

To do that, we should first create a new 1.2.x branch from 1.x.x and later do the regular backporting process.

What do you think?

Edit: correct branch names.


(Fatih) #7

Hi Marco,

All sounds good, do you mind creating the branch 1.1.x then I can fork it? (Assuming i don’t have rights to do so)

Cheers


(Marcos Pereira) #8

@fatihi

There you go: https://github.com/playframework/play-ws/tree/1.2.x

You can submit the PR against master branch and we can later backport the change. That way, we ensure the changes are also made accordingly for master/next-versions.

Best.