You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2011/04/30 12:26:13 UTC
svn commit: r1098093 - in
/httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http:
conn/params/ConnManagerParams.java impl/client/DefaultRequestDirector.java
Author: olegk
Date: Sat Apr 30 10:26:13 2011
New Revision: 1098093
URL: http://svn.apache.org/viewvc?rev=1098093&view=rev
Log:
Re-introduced timeout parameter for connection manager operations
Modified:
httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/conn/params/ConnManagerParams.java
httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java
Modified: httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/conn/params/ConnManagerParams.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/conn/params/ConnManagerParams.java?rev=1098093&r1=1098092&r2=1098093&view=diff
==============================================================================
--- httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/conn/params/ConnManagerParams.java (original)
+++ httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/conn/params/ConnManagerParams.java Sat Apr 30 10:26:13 2011
@@ -30,6 +30,7 @@ import org.apache.http.annotation.Immuta
import org.apache.http.conn.routing.HttpRoute;
import org.apache.http.impl.conn.tsccm.ThreadSafeClientConnManager;
+import org.apache.http.params.CoreConnectionPNames;
import org.apache.http.params.HttpConnectionParams;
import org.apache.http.params.HttpParams;
@@ -62,7 +63,11 @@ public final class ConnManagerParams imp
if (params == null) {
throw new IllegalArgumentException("HTTP parameters may not be null");
}
- return params.getLongParameter(TIMEOUT, 0);
+ Long param = (Long) params.getParameter(TIMEOUT);
+ if (param != null) {
+ return param.longValue();
+ }
+ return params.getIntParameter(CoreConnectionPNames.CONNECTION_TIMEOUT, 0);
}
/**
Modified: httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java?rev=1098093&r1=1098092&r2=1098093&view=diff
==============================================================================
--- httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java (original)
+++ httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java Sat Apr 30 10:26:13 2011
@@ -74,6 +74,7 @@ import org.apache.http.conn.ClientConnec
import org.apache.http.conn.ClientConnectionRequest;
import org.apache.http.conn.ConnectionKeepAliveStrategy;
import org.apache.http.conn.ManagedClientConnection;
+import org.apache.http.conn.params.ConnManagerParams;
import org.apache.http.conn.routing.BasicRouteDirector;
import org.apache.http.conn.routing.HttpRoute;
import org.apache.http.conn.routing.HttpRouteDirector;
@@ -130,6 +131,7 @@ import org.apache.http.util.EntityUtils;
*
* @since 4.0
*/
+@SuppressWarnings("deprecation")
@NotThreadSafe // e.g. managedConn
public class DefaultRequestDirector implements RequestDirector {
@@ -360,8 +362,6 @@ public class DefaultRequestDirector impl
RoutedRequest roureq = new RoutedRequest(origWrapper, origRoute);
- long timeout = HttpConnectionParams.getConnectionTimeout(params);
-
boolean reuse = false;
boolean done = false;
try {
@@ -387,6 +387,7 @@ public class DefaultRequestDirector impl
((AbortableHttpRequest) orig).setConnectionRequest(connRequest);
}
+ long timeout = ConnManagerParams.getTimeout(params);
try {
managedConn = connRequest.getConnection(timeout, TimeUnit.MILLISECONDS);
} catch(InterruptedException interrupted) {
Re: svn commit: r1098093
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Tue, 2011-05-03 at 20:39 +0100, sebb wrote:
> On 3 May 2011 20:22, Oleg Kalnichevski <ol...@apache.org> wrote:
> >
> >> I'm not sure I understand why the 4.1.x changes are different from the
> >> trunk changes.
> >>
> >
> > Ideally 0.0.x releases should be fully compatible so one could upgrade /
> > downgrade patch releases without having to recompile and redeploy. That
> > basically means there can be no new methods / classes in 0.0.x releases
>
> I see.
>
> I had not realised point releases were supposed to be backwards compatible.
>
They do not _really_ have to be, but something which I consider worth
having.
> >
> >> Why does ConnManagerParams.getTimeout - above - now return
> >> "http.connection.timeout" if "http.conn-manager.timeout" is not
> >> defined?
> >>
> >
> > This is the only way I could think of to preserve behavioral
> > compatibility with 4.1.0 and 4.1.1 releases
> >
> >> If a default is to be applied, should that not be done in
> >> DefaultRequestDirector instead?
> >>
> >
> > Fair point. I would not bother with the 4.1.x branch but certainly could
> > be done in trunk.
>
> Even though the fix changes the behaviour of getTimeout() ?
>
The method is deprecated. And really can't think of any side effects.
Feel free to move that logic to DefaultRequestDirector, though.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1098093
Posted by sebb <se...@gmail.com>.
On 3 May 2011 20:22, Oleg Kalnichevski <ol...@apache.org> wrote:
>
>> I'm not sure I understand why the 4.1.x changes are different from the
>> trunk changes.
>>
>
> Ideally 0.0.x releases should be fully compatible so one could upgrade /
> downgrade patch releases without having to recompile and redeploy. That
> basically means there can be no new methods / classes in 0.0.x releases
I see.
I had not realised point releases were supposed to be backwards compatible.
>
>> Why does ConnManagerParams.getTimeout - above - now return
>> "http.connection.timeout" if "http.conn-manager.timeout" is not
>> defined?
>>
>
> This is the only way I could think of to preserve behavioral
> compatibility with 4.1.0 and 4.1.1 releases
>
>> If a default is to be applied, should that not be done in
>> DefaultRequestDirector instead?
>>
>
> Fair point. I would not bother with the 4.1.x branch but certainly could
> be done in trunk.
Even though the fix changes the behaviour of getTimeout() ?
> Oleg
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1098093
Posted by Oleg Kalnichevski <ol...@apache.org>.
> I'm not sure I understand why the 4.1.x changes are different from the
> trunk changes.
>
Ideally 0.0.x releases should be fully compatible so one could upgrade /
downgrade patch releases without having to recompile and redeploy. That
basically means there can be no new methods / classes in 0.0.x releases
> Why does ConnManagerParams.getTimeout - above - now return
> "http.connection.timeout" if "http.conn-manager.timeout" is not
> defined?
>
This is the only way I could think of to preserve behavioral
compatibility with 4.1.0 and 4.1.1 releases
> If a default is to be applied, should that not be done in
> DefaultRequestDirector instead?
>
Fair point. I would not bother with the 4.1.x branch but certainly could
be done in trunk.
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: svn commit: r1098093 - in /httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http:
conn/params/ConnManagerParams.java impl/client/DefaultRequestDirector.java
Posted by sebb <se...@gmail.com>.
On 30 April 2011 11:26, <ol...@apache.org> wrote:
> Author: olegk
> Date: Sat Apr 30 10:26:13 2011
> New Revision: 1098093
>
> URL: http://svn.apache.org/viewvc?rev=1098093&view=rev
> Log:
> Re-introduced timeout parameter for connection manager operations
>
> Modified:
> httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/conn/params/ConnManagerParams.java
> httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java
>
> Modified: httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/conn/params/ConnManagerParams.java
> URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/conn/params/ConnManagerParams.java?rev=1098093&r1=1098092&r2=1098093&view=diff
> ==============================================================================
> --- httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/conn/params/ConnManagerParams.java (original)
> +++ httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/conn/params/ConnManagerParams.java Sat Apr 30 10:26:13 2011
> @@ -30,6 +30,7 @@ import org.apache.http.annotation.Immuta
>
> import org.apache.http.conn.routing.HttpRoute;
> import org.apache.http.impl.conn.tsccm.ThreadSafeClientConnManager;
> +import org.apache.http.params.CoreConnectionPNames;
> import org.apache.http.params.HttpConnectionParams;
> import org.apache.http.params.HttpParams;
>
> @@ -62,7 +63,11 @@ public final class ConnManagerParams imp
> if (params == null) {
> throw new IllegalArgumentException("HTTP parameters may not be null");
> }
> - return params.getLongParameter(TIMEOUT, 0);
> + Long param = (Long) params.getParameter(TIMEOUT);
> + if (param != null) {
> + return param.longValue();
> + }
> + return params.getIntParameter(CoreConnectionPNames.CONNECTION_TIMEOUT, 0);
> }
I'm not sure I understand why the 4.1.x changes are different from the
trunk changes.
Why does ConnManagerParams.getTimeout - above - now return
"http.connection.timeout" if "http.conn-manager.timeout" is not
defined?
If a default is to be applied, should that not be done in
DefaultRequestDirector instead?
In any case, I think the Javadoc needs to be updated to say what
parameters are involved.
> /**
>
> Modified: httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java
> URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java?rev=1098093&r1=1098092&r2=1098093&view=diff
> ==============================================================================
> --- httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java (original)
> +++ httpcomponents/httpclient/branches/4.1.x/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java Sat Apr 30 10:26:13 2011
> @@ -74,6 +74,7 @@ import org.apache.http.conn.ClientConnec
> import org.apache.http.conn.ClientConnectionRequest;
> import org.apache.http.conn.ConnectionKeepAliveStrategy;
> import org.apache.http.conn.ManagedClientConnection;
> +import org.apache.http.conn.params.ConnManagerParams;
> import org.apache.http.conn.routing.BasicRouteDirector;
> import org.apache.http.conn.routing.HttpRoute;
> import org.apache.http.conn.routing.HttpRouteDirector;
> @@ -130,6 +131,7 @@ import org.apache.http.util.EntityUtils;
> *
> * @since 4.0
> */
> +@SuppressWarnings("deprecation")
> @NotThreadSafe // e.g. managedConn
> public class DefaultRequestDirector implements RequestDirector {
>
> @@ -360,8 +362,6 @@ public class DefaultRequestDirector impl
>
> RoutedRequest roureq = new RoutedRequest(origWrapper, origRoute);
>
> - long timeout = HttpConnectionParams.getConnectionTimeout(params);
> -
> boolean reuse = false;
> boolean done = false;
> try {
> @@ -387,6 +387,7 @@ public class DefaultRequestDirector impl
> ((AbortableHttpRequest) orig).setConnectionRequest(connRequest);
> }
>
> + long timeout = ConnManagerParams.getTimeout(params);
> try {
> managedConn = connRequest.getConnection(timeout, TimeUnit.MILLISECONDS);
> } catch(InterruptedException interrupted) {
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org