Ivy - BasicURLHandler ignoring timeout during connection?

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Ivy - BasicURLHandler ignoring timeout during connection?

Jaikiran Pai
I was looking at some timing issues in test cases and noticed that the BasicURLHandler.getURLInfo with a timeout[1] seems to be ignoring that timeout completely. Am I missing something or is it just a oversight/bug? Furthermore, the javadoc of URLHandler.getURLInfo doesn’t tell much about what the timeout is about. I’m guessing it’s a connect timeout? Is the intention to use to same for (socket) read timeout too?

It’s another matter that the test case that I was looking into doesn’t pass a timeout.

[1] https://github.com/apache/ant-ivy/blob/master/src/java/org/apache/ivy/util/url/BasicURLHandler.java#L57
[2] https://github.com/apache/ant-ivy/blob/master/src/java/org/apache/ivy/util/url/URLHandler.java#L164

-Jaikiran
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Ivy - BasicURLHandler ignoring timeout during connection?

Gintautas Grigelionis
My 2¢: timeout has only effect on doHead, in all other cases Ivy is trying
the hardest it can. Just search for setTimeout :-)


2017-05-19 16:10 GMT+02:00 J Pai <[hidden email]>:

> I was looking at some timing issues in test cases and noticed that the
> BasicURLHandler.getURLInfo with a timeout[1] seems to be ignoring that
> timeout completely. Am I missing something or is it just a oversight/bug?
> Furthermore, the javadoc of URLHandler.getURLInfo doesn’t tell much about
> what the timeout is about. I’m guessing it’s a connect timeout? Is the
> intention to use to same for (socket) read timeout too?
>
> It’s another matter that the test case that I was looking into doesn’t
> pass a timeout.
>
> [1] https://github.com/apache/ant-ivy/blob/master/src/java/org/
> apache/ivy/util/url/BasicURLHandler.java#L57
> [2] https://github.com/apache/ant-ivy/blob/master/src/java/org/
> apache/ivy/util/url/URLHandler.java#L164
>
> -Jaikiran
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Ivy - BasicURLHandler ignoring timeout during connection?

Nicolas Lalevée
In reply to this post by Jaikiran Pai
I don’t know the history of that code, it probably is older that its incubation into Apache.
But from what I can read, I think that timeout was introduced but just supported by one implementation of URLHandler: the HttpClient one, HttpClientHandler.

Proper support in the BasicURLHandler will probably be welcomed.

Note though that a quick search in the call hierarchy shows that is is not used anywhere other than in IBiblioHelper.

So a proper support for timeout will probably require to propagate a timeout value up to the ivy settings, while declaring resolvers. And as you pointed, better semantic would need to be defined. Probably two kind of timeout should be defined.

Nicolas

> Le 19 mai 2017 à 16:10, J Pai <[hidden email]> a écrit :
>
> I was looking at some timing issues in test cases and noticed that the BasicURLHandler.getURLInfo with a timeout[1] seems to be ignoring that timeout completely. Am I missing something or is it just a oversight/bug? Furthermore, the javadoc of URLHandler.getURLInfo doesn’t tell much about what the timeout is about. I’m guessing it’s a connect timeout? Is the intention to use to same for (socket) read timeout too?
>
> It’s another matter that the test case that I was looking into doesn’t pass a timeout.
>
> [1] https://github.com/apache/ant-ivy/blob/master/src/java/org/apache/ivy/util/url/BasicURLHandler.java#L57
> [2] https://github.com/apache/ant-ivy/blob/master/src/java/org/apache/ivy/util/url/URLHandler.java#L164
>
> -Jaikiran
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Ivy - BasicURLHandler ignoring timeout during connection?

Gintautas Grigelionis
Looks like HttpClientHandler was added as an alternative to handle proxies
with authentication by wrapping HttpClient from Commons.

There is a maintenance problem because HttpClient has a major API change
between version 3 (that Ivy uses currently) and 4. Perhaps it makes sense
to look at AsyncHttpClient/Netty or should we wait for HttpClient version 5
(which is progressing through alpha releases currently)? Either way, we'll
be moving towards Java 8.

Meanwhile, it makes sense to improve BasicURLHandler and add such
capabilities as JRE permits.

Gintas

2017-05-20 11:46 GMT+02:00 Nicolas Lalevée <[hidden email]>:

> I don’t know the history of that code, it probably is older that its
> incubation into Apache.
> But from what I can read, I think that timeout was introduced but just
> supported by one implementation of URLHandler: the HttpClient one,
> HttpClientHandler.
>
> Proper support in the BasicURLHandler will probably be welcomed.
>
> Note though that a quick search in the call hierarchy shows that is is not
> used anywhere other than in IBiblioHelper.
>
> So a proper support for timeout will probably require to propagate a
> timeout value up to the ivy settings, while declaring resolvers. And as you
> pointed, better semantic would need to be defined. Probably two kind of
> timeout should be defined.
>
> Nicolas
>
> > Le 19 mai 2017 à 16:10, J Pai <[hidden email]> a écrit :
> >
> > I was looking at some timing issues in test cases and noticed that the
> BasicURLHandler.getURLInfo with a timeout[1] seems to be ignoring that
> timeout completely. Am I missing something or is it just a oversight/bug?
> Furthermore, the javadoc of URLHandler.getURLInfo doesn’t tell much about
> what the timeout is about. I’m guessing it’s a connect timeout? Is the
> intention to use to same for (socket) read timeout too?
> >
> > It’s another matter that the test case that I was looking into doesn’t
> pass a timeout.
> >
> > [1] https://github.com/apache/ant-ivy/blob/master/src/java/org/
> apache/ivy/util/url/BasicURLHandler.java#L57
> > [2] https://github.com/apache/ant-ivy/blob/master/src/java/org/
> apache/ivy/util/url/URLHandler.java#L164
> >
> > -Jaikiran
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Ivy - BasicURLHandler ignoring timeout during connection?

Jaikiran Pai
Having been involved in a HttpClient 3 to 4 migration in an unrelated project, I think at this point, it’s too big a change to consider. A few stable Ivy releases down the line, maybe we can re-evaluate this and consider the HttpClient libraries and their state at that time.

-Jaikiran
On 20-May-2017, at 6:33 PM, Gintautas Grigelionis <[hidden email]> wrote:

Looks like HttpClientHandler was added as an alternative to handle proxies
with authentication by wrapping HttpClient from Commons.

There is a maintenance problem because HttpClient has a major API change
between version 3 (that Ivy uses currently) and 4. Perhaps it makes sense
to look at AsyncHttpClient/Netty or should we wait for HttpClient version 5
(which is progressing through alpha releases currently)? Either way, we'll
be moving towards Java 8.

Meanwhile, it makes sense to improve BasicURLHandler and add such
capabilities as JRE permits.

Gintas

2017-05-20 11:46 GMT+02:00 Nicolas Lalevée <[hidden email]>:

> I don’t know the history of that code, it probably is older that its
> incubation into Apache.
> But from what I can read, I think that timeout was introduced but just
> supported by one implementation of URLHandler: the HttpClient one,
> HttpClientHandler.
>
> Proper support in the BasicURLHandler will probably be welcomed.
>
> Note though that a quick search in the call hierarchy shows that is is not
> used anywhere other than in IBiblioHelper.
>
> So a proper support for timeout will probably require to propagate a
> timeout value up to the ivy settings, while declaring resolvers. And as you
> pointed, better semantic would need to be defined. Probably two kind of
> timeout should be defined.
>
> Nicolas
>
>> Le 19 mai 2017 à 16:10, J Pai <[hidden email]> a écrit :
>>
>> I was looking at some timing issues in test cases and noticed that the
> BasicURLHandler.getURLInfo with a timeout[1] seems to be ignoring that
> timeout completely. Am I missing something or is it just a oversight/bug?
> Furthermore, the javadoc of URLHandler.getURLInfo doesn’t tell much about
> what the timeout is about. I’m guessing it’s a connect timeout? Is the
> intention to use to same for (socket) read timeout too?
>>
>> It’s another matter that the test case that I was looking into doesn’t
> pass a timeout.
>>
>> [1] https://github.com/apache/ant-ivy/blob/master/src/java/org/
> apache/ivy/util/url/BasicURLHandler.java#L57
>> [2] https://github.com/apache/ant-ivy/blob/master/src/java/org/
> apache/ivy/util/url/URLHandler.java#L164
>>
>> -Jaikiran
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Ivy - BasicURLHandler ignoring timeout during connection?

Gintautas Grigelionis
I was merely speculating as to why HttpClient is there and what challenges
lay ahead should that part of code be refreshed.

Sorry for digressing, adding setConnectTimeout is fine (even though
setTimeout from HttpClient corresponds to setReadTimeout in URLConnection
-- perhaps that should be changed in HttpClientHandler for consistency?).

Gintas

2017-05-21 7:00 GMT+02:00 J Pai <[hidden email]>:

> Having been involved in a HttpClient 3 to 4 migration in an unrelated
> project, I think at this point, it’s too big a change to consider. A few
> stable Ivy releases down the line, maybe we can re-evaluate this and
> consider the HttpClient libraries and their state at that time.
>
> -Jaikiran
> On 20-May-2017, at 6:33 PM, Gintautas Grigelionis <[hidden email]>
> wrote:
>
> Looks like HttpClientHandler was added as an alternative to handle proxies
> with authentication by wrapping HttpClient from Commons.
>
> There is a maintenance problem because HttpClient has a major API change
> between version 3 (that Ivy uses currently) and 4. Perhaps it makes sense
> to look at AsyncHttpClient/Netty or should we wait for HttpClient version 5
> (which is progressing through alpha releases currently)? Either way, we'll
> be moving towards Java 8.
>
> Meanwhile, it makes sense to improve BasicURLHandler and add such
> capabilities as JRE permits.
>
> Gintas
>
> 2017-05-20 11:46 GMT+02:00 Nicolas Lalevée <[hidden email]>:
>
> > I don’t know the history of that code, it probably is older that its
> > incubation into Apache.
> > But from what I can read, I think that timeout was introduced but just
> > supported by one implementation of URLHandler: the HttpClient one,
> > HttpClientHandler.
> >
> > Proper support in the BasicURLHandler will probably be welcomed.
> >
> > Note though that a quick search in the call hierarchy shows that is is
> not
> > used anywhere other than in IBiblioHelper.
> >
> > So a proper support for timeout will probably require to propagate a
> > timeout value up to the ivy settings, while declaring resolvers. And as
> you
> > pointed, better semantic would need to be defined. Probably two kind of
> > timeout should be defined.
> >
> > Nicolas
> >
> >> Le 19 mai 2017 à 16:10, J Pai <[hidden email]> a écrit :
> >>
> >> I was looking at some timing issues in test cases and noticed that the
> > BasicURLHandler.getURLInfo with a timeout[1] seems to be ignoring that
> > timeout completely. Am I missing something or is it just a oversight/bug?
> > Furthermore, the javadoc of URLHandler.getURLInfo doesn’t tell much about
> > what the timeout is about. I’m guessing it’s a connect timeout? Is the
> > intention to use to same for (socket) read timeout too?
> >>
> >> It’s another matter that the test case that I was looking into doesn’t
> > pass a timeout.
> >>
> >> [1] https://github.com/apache/ant-ivy/blob/master/src/java/org/
> > apache/ivy/util/url/BasicURLHandler.java#L57
> >> [2] https://github.com/apache/ant-ivy/blob/master/src/java/org/
> > apache/ivy/util/url/URLHandler.java#L164
> >>
> >> -Jaikiran
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: [hidden email]
> >> For additional commands, e-mail: [hidden email]
> >>
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Ivy - BasicURLHandler ignoring timeout during connection?

Jaikiran Pai

>>I was merely speculating as to why HttpClient is there and what challenges
>> lay ahead should that part of code be refreshed.

Understood :)

>> Sorry for digressing, adding setConnectTimeout is fine

I sent a PR with minimal changes for now https://github.com/apache/ant-ivy/pull/23

>> (even though setTimeout from HttpClient corresponds to setReadTimeout in URLConnection
>> -- perhaps that should be changed in HttpClientHandler for consistency?).

I like what Nicolas suggested in one of his replies, that we make this (and other necessary timeout configs) a well specified contract and have them defined all the way from the settings. I think we can handle this change as part of that work. I can file a enhancement JIRA to track this work and have it available in either the upcoming release or some release after that (depending on how big this change turns out to be).

My main motivation behind BasicURLHandler at this point is the fact that one of the test which uses this class takes unnecessarily long because of the missing timeout support. The minor change that I included in that PR should let us move forward, on that front.

-Jaikiran

2017-05-21 7:00 GMT+02:00 J Pai <[hidden email]>:

> Having been involved in a HttpClient 3 to 4 migration in an unrelated
> project, I think at this point, it’s too big a change to consider. A few
> stable Ivy releases down the line, maybe we can re-evaluate this and
> consider the HttpClient libraries and their state at that time.
>
> -Jaikiran
> On 20-May-2017, at 6:33 PM, Gintautas Grigelionis <[hidden email]>
> wrote:
>
> Looks like HttpClientHandler was added as an alternative to handle proxies
> with authentication by wrapping HttpClient from Commons.
>
> There is a maintenance problem because HttpClient has a major API change
> between version 3 (that Ivy uses currently) and 4. Perhaps it makes sense
> to look at AsyncHttpClient/Netty or should we wait for HttpClient version 5
> (which is progressing through alpha releases currently)? Either way, we'll
> be moving towards Java 8.
>
> Meanwhile, it makes sense to improve BasicURLHandler and add such
> capabilities as JRE permits.
>
> Gintas
>
> 2017-05-20 11:46 GMT+02:00 Nicolas Lalevée <[hidden email]>:
>
>> I don’t know the history of that code, it probably is older that its
>> incubation into Apache.
>> But from what I can read, I think that timeout was introduced but just
>> supported by one implementation of URLHandler: the HttpClient one,
>> HttpClientHandler.
>>
>> Proper support in the BasicURLHandler will probably be welcomed.
>>
>> Note though that a quick search in the call hierarchy shows that is is
> not
>> used anywhere other than in IBiblioHelper.
>>
>> So a proper support for timeout will probably require to propagate a
>> timeout value up to the ivy settings, while declaring resolvers. And as
> you
>> pointed, better semantic would need to be defined. Probably two kind of
>> timeout should be defined.
>>
>> Nicolas
>>
>>> Le 19 mai 2017 à 16:10, J Pai <[hidden email]> a écrit :
>>>
>>> I was looking at some timing issues in test cases and noticed that the
>> BasicURLHandler.getURLInfo with a timeout[1] seems to be ignoring that
>> timeout completely. Am I missing something or is it just a oversight/bug?
>> Furthermore, the javadoc of URLHandler.getURLInfo doesn’t tell much about
>> what the timeout is about. I’m guessing it’s a connect timeout? Is the
>> intention to use to same for (socket) read timeout too?
>>>
>>> It’s another matter that the test case that I was looking into doesn’t
>> pass a timeout.
>>>
>>> [1] https://github.com/apache/ant-ivy/blob/master/src/java/org/
>> apache/ivy/util/url/BasicURLHandler.java#L57
>>> [2] https://github.com/apache/ant-ivy/blob/master/src/java/org/
>> apache/ivy/util/url/URLHandler.java#L164
>>>
>>> -Jaikiran
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: [hidden email]
>>> For additional commands, e-mail: [hidden email]
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]