Implement configurable connection timeouts on the DefaultHttpProvider #190
Conversation
|
Could you guys take a look at this and see if it makes sense. |
Krisoblucki-okta
force-pushed
the
configurable-timeout-in-url-connect
branch
2 times, most recently
from
1aea835 to
9a1d3e9
Compare
Krisoblucki-okta
force-pushed
the
configurable-timeout-in-url-connect
branch
from
c0d4da4 to
a73cdb8
Compare
.gitignore
Outdated
| .idea | ||
|
|
||
| # Output | ||
| /out |
There was a problem hiding this comment.
Please revert changes to this file.
| /** | ||
| * Default connection read timeout | ||
| */ | ||
| private static final int DEFAULT_READ_TIMEOUT_MS = 30_000; |
There was a problem hiding this comment.
Remove final, by keeping it final it can never be changed and the value will be always 30 _000.
| /** | ||
| * Default connect timeout | ||
| */ | ||
| private static final int DEFAULT_CONNECT_TIMEOUT_MS = 30_000; |
There was a problem hiding this comment.
Remove final, by keeping it final it can never be changed and the value will be always 30 _000.
| public int getConnectTimeout() { | ||
| return DEFAULT_CONNECT_TIMEOUT_MS; | ||
| } | ||
|
|
There was a problem hiding this comment.
Provide setter for DEFAULT_CONNECT_TIMEOUT_MS.
| @Override | ||
| public int getReadTimeout() { | ||
| return DEFAULT_READ_TIMEOUT_MS; | ||
| } |
There was a problem hiding this comment.
Provide setter for DEFAULT_READ_TIMEOUT_MS.
| * @return the timeout in milliseconds | ||
| */ | ||
| int getReadTimeout(); | ||
| } No newline at end of file |
There was a problem hiding this comment.
Provide void setReadTimeout(); so that change of DEFAULT_READ_TIMEOUT_MS is possible.
| /** | ||
| * The connection config | ||
| */ | ||
| private final IConnectionConfig connectionConfig; |
There was a problem hiding this comment.
Remove final from here, so that any class implementing IConnectionConfig can be used rather depending on fixed DefaultConnectionConfig.
| * The connection config | ||
| */ | ||
| private final IConnectionConfig connectionConfig; | ||
|
|
There was a problem hiding this comment.
Provide getter and setter for connectionConfig in DefaultHttpProvider and IHttpProvider.
| final IExecutors executors, | ||
| final ILogger logger, | ||
| final IConnectionConfig connectionConfig | ||
| ) { |
There was a problem hiding this comment.
Adding a new constructor is not required.
| final URL requestUrl = request.getRequestUrl(); | ||
| logger.logDebug("Starting to send request, URL " + requestUrl.toString()); | ||
| final IConnection connection = connectionFactory.createFromRequest(request); | ||
| connection.setConnectTimeout(connectionConfig.getConnectTimeout()); |
There was a problem hiding this comment.
Use this
if(this.connectionConfig == null) {
this.connectionConfig = new DefaultConnectionConfig();
}
So, that default values are only used when config is null.
This connection.setConnectTimeout(connectionConfig.getConnectTimeout()); is fine.
| logger.logDebug("Starting to send request, URL " + requestUrl.toString()); | ||
| final IConnection connection = connectionFactory.createFromRequest(request); | ||
| connection.setConnectTimeout(connectionConfig.getConnectTimeout()); | ||
| connection.setReadTimeout(connectionConfig.getReadTimeout()); |
|
Hi, DefaultConnectionConfig will contain the values of timeout which can be changed using set and get. This config will be passed using getter and setter in DefaultHttpProvider to change value of IConnectionConfig connectionConfig. Timeout and connectionConfig cannot be made final as they can be changed. |
|
We have fixed the issue using PR: #216 |
3 participants
Currently there is no way to configure the connection timeouts to custom values as they are hard coded. This change allows connection timeouts to be configured.