You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by garydgregory <gi...@git.apache.org> on 2018/10/12 16:28:58 UTC
[GitHub] httpcomponents-core pull request #90: Define a timeout exception more precis...
GitHub user garydgregory opened a pull request:
https://github.com/apache/httpcomponents-core/pull/90
Define a timeout exception more precisely.
Create and use a subclass of java.util.concurrent.TimeoutException called TimeoutValueException to precisely define a timeout exception.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/garydgregory/httpcore timeout-value-exception
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/httpcomponents-core/pull/90.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #90
----
commit 64d3b53437342a1cdeb72df08368392406e37098
Author: Gary Gregory <gg...@...>
Date: 2018-10-12T16:26:14Z
Create a subclass of java.util.concurrent.TimeoutException called TimeoutValueException to precisely define a timeout exception.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpcomponents-core pull request #90: Define a timeout exception more precis...
Posted by ok2c <gi...@git.apache.org>.
Github user ok2c commented on a diff in the pull request:
https://github.com/apache/httpcomponents-core/pull/90#discussion_r224967357
--- Diff: httpcore5/src/main/java/org/apache/hc/core5/concurrent/BasicFuture.java ---
@@ -94,7 +95,7 @@ public synchronized T get(final long timeout, final TimeUnit unit)
if (this.completed) {
return getResult();
} else if (waitTime <= 0) {
- throw new TimeoutException();
+ throw TimeoutValueException.ofMillis(msecs, msecs + Math.abs(waitTime));
--- End diff --
@garydgregory This is clearly not how I interpret this piece of code.
```
final long now = System.currentTimeMillis();
if (now > deadline) {
leaseRequest.failed(TimeoutValueException.ofMillis(deadline, now));
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpcomponents-core pull request #90: Define a timeout exception more precis...
Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/httpcomponents-core/pull/90#discussion_r224965993
--- Diff: httpcore5/src/main/java/org/apache/hc/core5/concurrent/BasicFuture.java ---
@@ -94,7 +95,7 @@ public synchronized T get(final long timeout, final TimeUnit unit)
if (this.completed) {
return getResult();
} else if (waitTime <= 0) {
- throw new TimeoutException();
+ throw TimeoutValueException.ofMillis(msecs, msecs + Math.abs(waitTime));
--- End diff --
Hi @ok2c ,
The 'actual' value is _not_ the number of milliseconds that have elapsed since midnight, January 1, 1970. It is a value >= deadline relative to the deadline because 'msecs' is equal to the timeout value in millis. The 'actual' value is really 'msecs' plus how much over the deadline we waited represented by 'waitTime'. This is useful because if the deadline is 30 seconds, but we ended up waiting 5 minutes, then we or the user can look for what I claim would be a bug, depending on how the Future is used.
Gary
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpcomponents-core pull request #90: Define a timeout exception and deadlin...
Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/httpcomponents-core/pull/90#discussion_r225022740
--- Diff: httpcore5/src/main/java/org/apache/hc/core5/concurrent/BasicFuture.java ---
@@ -94,7 +95,7 @@ public synchronized T get(final long timeout, final TimeUnit unit)
if (this.completed) {
return getResult();
} else if (waitTime <= 0) {
- throw new TimeoutException();
+ throw TimeoutValueException.ofMillis(msecs, msecs + Math.abs(waitTime));
--- End diff --
@ok2c I've updated this PR to fix the above issue and also refactored our use of a deadline long and its redundant code into a new Deadline class.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpcomponents-core pull request #90: Define a timeout exception more precis...
Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/httpcomponents-core/pull/90#discussion_r225006254
--- Diff: httpcore5/src/main/java/org/apache/hc/core5/concurrent/BasicFuture.java ---
@@ -94,7 +95,7 @@ public synchronized T get(final long timeout, final TimeUnit unit)
if (this.completed) {
return getResult();
} else if (waitTime <= 0) {
- throw new TimeoutException();
+ throw TimeoutValueException.ofMillis(msecs, msecs + Math.abs(waitTime));
--- End diff --
Yes, for that call site, you are correct, my implementation is incorrect, I will adjust...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpcomponents-core issue #90: Define a timeout exception and deadline more ...
Posted by ok2c <gi...@git.apache.org>.
Github user ok2c commented on the issue:
https://github.com/apache/httpcomponents-core/pull/90
@garydgregory There is one test case failing in Travis CI with all JDKs. Could you please have a look?
I suggest this change gets included in the next release. I'll proceed with 5.0 BETA4 in the meantime.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpcomponents-core issue #90: Define a timeout exception and deadline more ...
Posted by ok2c <gi...@git.apache.org>.
Github user ok2c commented on the issue:
https://github.com/apache/httpcomponents-core/pull/90
Committed as b77420f. Please review and close this PR.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpcomponents-core issue #90: Define a timeout exception and deadline more ...
Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory commented on the issue:
https://github.com/apache/httpcomponents-core/pull/90
@ok2c With my latest commits, all Travis CI builds are green for this PR.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpcomponents-core pull request #90: Define a timeout exception more precis...
Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/httpcomponents-core/pull/90#discussion_r224963982
--- Diff: httpcore5/src/main/java/org/apache/hc/core5/concurrent/BasicFuture.java ---
@@ -94,7 +95,7 @@ public synchronized T get(final long timeout, final TimeUnit unit)
if (this.completed) {
return getResult();
} else if (waitTime <= 0) {
- throw new TimeoutException();
+ throw TimeoutValueException.ofMillis(msecs, msecs + Math.abs(waitTime));
--- End diff --
@ok2c ,
Thank you for your comment.
What is the "timeout value"? Is it the boundary of time above which the timeout occurs or is it the actual elapsed time? To both avoid this confusion and to be as precise with the message as possible, both values are given:
- The "deadline" in the message is the the boundary of time above which the timeout occurs.
- The "actual" in the message is the actual elapsed time.
Does that clarify things for you?
Gary
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpcomponents-core issue #90: Define a timeout exception and deadline more ...
Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory commented on the issue:
https://github.com/apache/httpcomponents-core/pull/90
PR merged; local build OK; closing.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpcomponents-core pull request #90: Define a timeout exception more precis...
Posted by ok2c <gi...@git.apache.org>.
Github user ok2c commented on a diff in the pull request:
https://github.com/apache/httpcomponents-core/pull/90#discussion_r224965432
--- Diff: httpcore5/src/main/java/org/apache/hc/core5/concurrent/BasicFuture.java ---
@@ -94,7 +95,7 @@ public synchronized T get(final long timeout, final TimeUnit unit)
if (this.completed) {
return getResult();
} else if (waitTime <= 0) {
- throw new TimeoutException();
+ throw TimeoutValueException.ofMillis(msecs, msecs + Math.abs(waitTime));
--- End diff --
@garydgregory To me it is Is it the _"boundary of time above which the timeout occurs"_. Yes, it does clarify but i personally do not find this information useful. The initial timeout value given by the user, say 30 seconds, might be marginally useful, but the number of milliseconds that have elapsed since midnight, January 1, 1970, when the operation timed out is not.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpcomponents-core pull request #90: Define a timeout exception more precis...
Posted by ok2c <gi...@git.apache.org>.
Github user ok2c commented on a diff in the pull request:
https://github.com/apache/httpcomponents-core/pull/90#discussion_r224955724
--- Diff: httpcore5/src/main/java/org/apache/hc/core5/concurrent/BasicFuture.java ---
@@ -94,7 +95,7 @@ public synchronized T get(final long timeout, final TimeUnit unit)
if (this.completed) {
return getResult();
} else if (waitTime <= 0) {
- throw new TimeoutException();
+ throw TimeoutValueException.ofMillis(msecs, msecs + Math.abs(waitTime));
--- End diff --
@garydgregory What is the value of this information? What good does it make to print out current time and deadline in milliseconds? I thought you were going to include the timeout value in the exception message, which would be of some marginal use.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpcomponents-core pull request #90: Define a timeout exception and deadlin...
Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory closed the pull request at:
https://github.com/apache/httpcomponents-core/pull/90
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [GitHub] httpcomponents-core pull request #90: Define a timeout
exception more precis...
Posted by Gary Gregory <ga...@gmail.com>.
On Fri, Oct 12, 2018 at 11:20 AM Gary Gregory <ga...@gmail.com>
wrote:
> Once this PR is hopefully accepted, I'd like to add and
> use org.apache.hc.core5.concurrent.BasicFuture.get(Timeout).
>
Hm, BasicFuture.get(Timeout) probably does not make sense
since java.util.concurrent.Future does not define such a get() method;
unless we want to create our own interface that extends Future to add a
get(Timeout).
Any thoughts on that one?
Gary
> Gary
>
> On Fri, Oct 12, 2018 at 10:28 AM garydgregory <gi...@git.apache.org> wrote:
>
>> GitHub user garydgregory opened a pull request:
>>
>> https://github.com/apache/httpcomponents-core/pull/90
>>
>> Define a timeout exception more precisely.
>>
>> Create and use a subclass of java.util.concurrent.TimeoutException
>> called TimeoutValueException to precisely define a timeout exception.
>>
>> You can merge this pull request into a Git repository by running:
>>
>> $ git pull https://github.com/garydgregory/httpcore
>> timeout-value-exception
>>
>> Alternatively you can review and apply these changes as the patch at:
>>
>> https://github.com/apache/httpcomponents-core/pull/90.patch
>>
>> To close this pull request, make a commit to your master/trunk branch
>> with (at least) the following in the commit message:
>>
>> This closes #90
>>
>> ----
>> commit 64d3b53437342a1cdeb72df08368392406e37098
>> Author: Gary Gregory <gg...@...>
>> Date: 2018-10-12T16:26:14Z
>>
>> Create a subclass of java.util.concurrent.TimeoutException called
>> TimeoutValueException to precisely define a timeout exception.
>>
>> ----
>>
>>
>> ---
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
>> For additional commands, e-mail: dev-help@hc.apache.org
>>
>>
Re: [GitHub] httpcomponents-core pull request #90: Define a timeout
exception more precis...
Posted by Gary Gregory <ga...@gmail.com>.
Once this PR is hopefully accepted, I'd like to add and
use org.apache.hc.core5.concurrent.BasicFuture.get(Timeout).
Gary
On Fri, Oct 12, 2018 at 10:28 AM garydgregory <gi...@git.apache.org> wrote:
> GitHub user garydgregory opened a pull request:
>
> https://github.com/apache/httpcomponents-core/pull/90
>
> Define a timeout exception more precisely.
>
> Create and use a subclass of java.util.concurrent.TimeoutException
> called TimeoutValueException to precisely define a timeout exception.
>
> You can merge this pull request into a Git repository by running:
>
> $ git pull https://github.com/garydgregory/httpcore
> timeout-value-exception
>
> Alternatively you can review and apply these changes as the patch at:
>
> https://github.com/apache/httpcomponents-core/pull/90.patch
>
> To close this pull request, make a commit to your master/trunk branch
> with (at least) the following in the commit message:
>
> This closes #90
>
> ----
> commit 64d3b53437342a1cdeb72df08368392406e37098
> Author: Gary Gregory <gg...@...>
> Date: 2018-10-12T16:26:14Z
>
> Create a subclass of java.util.concurrent.TimeoutException called
> TimeoutValueException to precisely define a timeout exception.
>
> ----
>
>
> ---
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
>
[GitHub] httpcomponents-core issue #90: Define a timeout exception more precisely.
Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory commented on the issue:
https://github.com/apache/httpcomponents-core/pull/90
@ok2c I will reply/work on it when I get home tonight (on my phone ATM.)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpcomponents-core issue #90: Define a timeout exception and deadline more ...
Posted by ok2c <gi...@git.apache.org>.
Github user ok2c commented on the issue:
https://github.com/apache/httpcomponents-core/pull/90
@garydgregory I'll merge this PR after HttpCore 5.0-beta4 gets tagged.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpcomponents-core issue #90: Define a timeout exception more precisely.
Posted by ok2c <gi...@git.apache.org>.
Github user ok2c commented on the issue:
https://github.com/apache/httpcomponents-core/pull/90
@garydgregory Are going to re-spin this patch? Shall I delay the release because of it?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] httpcomponents-core issue #90: Define a timeout exception and deadline more ...
Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory commented on the issue:
https://github.com/apache/httpcomponents-core/pull/90
@ok2c , OK, thank you.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org