On Wed, 14 Aug 2024 11:05:52 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Thats a good point, changing the timeunit to nanos is probably for the best.
>>
>> I wonder in the case of timeout being equal to 0 what the best way to handle
>> it is though?
>> perhaps something like
>> ```
>> long responseTimeout = TimeUnit.SECONDS.toNanos(5);
>> if (request.timeout().isPresent()
>> && request.timeout().get().getNano() > 0
>> && request.timeout().get().getNano() < responseTimeout) {
>> responseTimeout = request.timeout().get().getNano();
>> }
>
>> I wonder in the case of timeout being equal to 0 what the best way to handle
>> it is though?
>
> The `HttpRequest.Builder.timeout()` doesn't allow for `0` (or negative) value
> to be specified for the timeout duration. So if the request timeout is indeed
> specified, then we can be sure that the nanos representation of it here will
> be greater than 0. We could just add an `assert` for that, after converting
> it to nanos (when the request timeout is present). Something like this
> untested code:
>
>
> long timeoutNanos = TimeUnit.SECONDS.toNanos(5);
> if(request.timeout().isPresent() {
> final long rtNanos = request.timeout().get().getNano();
> assert rtNanos > 0 : "unexpected request timeout: " + rtNanos;
> timeoutNanos = Math.min(timeoutNanos, rt);
> });
I would just let 0 be 0: no need to split 1ms in 500_000 nanos :-)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20525#discussion_r1716739478
