Apache Http Client 5.x Use Timeout and TimeValue classes

[joanvr]

What's changed?

Using Timeout and TimeValue classes where previously an int (in milliseconds) was used.

What's your motivation?

In Apache Http Client 5.x the previously deprecated methods of timeouts and time durations with an integer parameter in milliseconds no longer exist.

Checklist

• I've added unit tests to cover both positive and negative cases
• I've added the license header to any new files through ./gradlew licenseFormat
• I've used the IntelliJ auto-formatter on affected files

[timtebeek]

Out of curiosity: Do these methods actually exist? Or only when we're halfway through a migration with change package having partially updated the classes such that we use the new package here, but the methods taking an int as an argument do not actually exist?

I'm wondering if the 4.x deprecated methods already had replacements taking a Timeout, which we could fully migrate in the 4.x line instead, before we even start a migration to 5.x. That could make the recipes more broadly applicable, and applied in isolation.

[joanvr]

Unfortunately, in 4.x all timeouts and tiemvalues are expressed with integers in milliseconds, and on 5.x there are two options, using Timeout and TimeValue classes, or long + TimeUnit... So, those methods we are matching here does not actually exists in the classes in the namespace, but are "half-way" partially updated mapped classes to 5.x

[rpau]

LGTM

[timtebeek]

Linking both classes here:
https://hc.apache.org/httpcomponents-core-4.4.x/current/httpcore/apidocs/org/apache/http/config/SocketConfig.Builder.html
https://hc.apache.org/httpcomponents-core-5.1.x/current/httpcore5/apidocs/org/apache/hc/core5/http/io/SocketConfig.Builder.html

Looking at the changes in those side by side I wonder why we went for the option of TimeValue/TimeOut, instead of adding a TimeUnit. Now setSoLinger and setTimeOut are in different recipes, whereas we could have used one recipe had we opted to add a TimeUnit argument instead, with less to maintain.

[joanvr]

Linking both classes here: https://hc.apache.org/httpcomponents-core-4.4.x/current/httpcore/apidocs/org/apache/http/config/SocketConfig.Builder.html https://hc.apache.org/httpcomponents-core-5.1.x/current/httpcore5/apidocs/org/apache/hc/core5/http/io/SocketConfig.Builder.html

Looking at the changes in those side by side I wonder why we went for the option of TimeValue/TimeOut, instead of adding a TimeUnit. Now setSoLinger and setTimeOut are in different recipes, whereas we could have used one recipe had we opted to add a TimeUnit argument instead, with less to maintain.

I can change this to one recipe and use TimeUnit instead if you prefer it, I've just sticked to the text in the migration guide that says:

• Use Timeout class to define timeouts.
• Use TimeValue class to define time values (duration).

I guess this way we are adding more semantic into the method call, explaining if it's a timeout or a duration. I think the other method call is there, to be similar to the original one, but aside from adding a unit, it has the same semantic information about it.