Add support for HttpClient requestTimeout param to relevant components
Description
Environment
Activity
Brent Putman December 6, 2024 at 3:50 PM
Wiki updates for the 2 new properties done.
Brent Putman December 6, 2024 at 3:27 PM
Per call discussion, we agreed to revert the default for the dynamic provider to null/disabled, like all the other use cases. Added some comments to enable in 6.0.
Scott Cantor December 6, 2024 at 2:57 PM
There might be some issue with the Duration converter, I’d have thought null might work as a String conversion. I can take a look, but yeah.
I understand that the new class is used, I meant that it wouldn’t do anything out of the box.
Brent Putman December 6, 2024 at 2:52 PM
Well, null is effectively “off”. That will result in the new machinery not being invoked.
As I mentioned in the other issue, it doesn’t mean that the new component isn’t produced by the builder, but the code path when the requestTimeout
is null won’t do anything. Its just essentially a pass-through in that case (unless one specified a per-request timeout via the client context, which we currently aren’t doing anywhere).
(And I think I figured out the SpEL wiring thing for a null default - I believe should be %{propName:#{null}}
. )
Scott Cantor December 6, 2024 at 12:58 PM
That seems like a reasonable/conservative default, though I saw that commit regarding allowing “null”. I thought our plan was just to default this new setting “off” so we wouldn’t risk any impact from the implementation and we could probably change it to something else for 6.0 once we know it works?
In our
HttpClientBuilder
we now produce a new customHttpClient
impl which supports an overall limit on the total request/response time. Add support for this in the parsers and schemas where relevant, i.e. where we have explicit support for the other timeout params.