Add support for HttpClient requestTimeout param to relevant components

Description

In our HttpClientBuilder we now produce a new custom HttpClient 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.

Environment

None

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?

Details

Assignee

Reporter

Fix versions

Created December 6, 2024 at 4:34 AM
Updated December 6, 2024 at 3:50 PM