Conversation
There was a problem hiding this comment.
Pull request overview
Updates the REST client’s configuration defaults to enforce sensible timeouts by default, addressing issue #434 and reducing the chance of indefinitely hanging requests/connections.
Changes:
- Set a default
responseTimeout(60 seconds) instead of leaving it unset. - Set default connection pool eviction settings via
maxIdleTime(200 seconds) andmaxLifeTime(1 hour).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rest-client-base/src/main/java/com/wultra/core/rest/client/base/RestClientConfiguration.java
Show resolved
Hide resolved
| private Duration responseTimeout = Duration.ofSeconds(60); | ||
|
|
||
| private Duration maxIdleTime; | ||
| private Duration maxLifeTime; | ||
| private Duration maxIdleTime = Duration.ofSeconds(200); | ||
| private Duration maxLifeTime = Duration.ofHours(1); |
There was a problem hiding this comment.
This introduces new default behavior (timeouts and connection pool maxIdleTime/maxLifeTime) but there is no test asserting these defaults when the user does not explicitly configure them. Consider adding/adjusting tests to verify the default durations are applied (and that they can still be disabled when set to null via the builder), to prevent accidental regressions.
There was a problem hiding this comment.
I agree we should have test for the default values
2 participants
No description provided.