-
Notifications
You must be signed in to change notification settings - Fork 26
Improve waiting strategy for Integration tests #399
Improve waiting strategy for Integration tests #399
Conversation
@@ -57,7 +51,7 @@ public static ApiClient getParodosAPiClient() | |||
} | |||
|
|||
int port = Integer.parseInt(serverPort); | |||
if (port <= 0 && port > 65535) { | |||
if (port <= 0 || port > 65535) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this isn't related to this PR, but fixes a bug since this if used to return always false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're right. Fixes even also e837ae6
*/ | ||
public T submitWithRetry(Callable<T> task) { | ||
// @formatter:off | ||
return submitWithRetry(task, () -> {}, () -> {}, 10 * 60 * 1000, 5000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gciavarrini what should be the timeout? was it used to be 1 minute in the previous implementation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the bash implementation there were 100 retries.
I chose 1 minute because it seemed like a reasonable time interval and I didn't receive any feedback on this.
TBH, I'm not sure which is the best timeout value :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Instead of relying on okhttp async implementation, we use executor service to invoke recurring requests to the service till completion. This is designed to decrease the CPU load even lower while maintaining more readable and easier-to-maintain code. Signed-off-by: Moti Asayag <masayag@redhat.com>
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gciavarrini The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm |
What this PR does / why we need it:
Instead of relying on okhttp async implementation, we use executor service to invoke recurring requests to the service till completion.
This is designed to decrease the CPU load even lower while maintaining more readable and easier-to-maintain code.
Which issue(s) this PR fixes (optional, use
fixes #<issue_number>(, fixes #<issue_number>, ...)
format, where issue_number might be a GitHub issue, or a Jira story (FLPATH-xxxx):Fixes #FLPATH-399
Change type
Impacted services
Checklist