Add retryOnReturnValues and retryOnReturnValuesExcluding #79
Add retryOnReturnValues and retryOnReturnValuesExcluding #79
Conversation
Pull Request Test Coverage Report for Build 345
💛 - Coveralls |
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.
sorry for the delay... left some feedback
@@ -153,6 +154,32 @@ private void postExecutionCleanup(Callable<T> callable, int maxTries, AttemptSta | |||
return attemptStatus; | |||
} | |||
|
|||
private boolean isOneOfValuesToExpect( T callResult ) { | |||
Object[] valuesToExpect = config.getValuesToExpect(); |
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.
could you format these blocks to be consistent with code styling in the rest of the class? eg: less whitespace and line breaks
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.
Updated.
|
||
@SafeVarargs | ||
public final RetryConfigBuilder retryOnReturnValues(Object... values) { | ||
retryOnValue = true; |
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.
i would generally prefer wrapping these with a Collection of some sort rather than storing them as a raw array... for example: https://github.com/elennick/retry4j/pull/79/files#diff-25b653eb79d0323ac9625faf0ae65ef9R101
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.
Fair enough. Updated.
Thanks for the review and I updated accordingly. |
@ruhan1 any chance you wouldn't mind updating the README as well to reflect the new API's you added? |
I need two more APIs in RetryConfigBuilder. The retryOnReturnValues takes an array of retriable values. The retryOnReturnValuesExcluding handles the case where I only expect certain return value(s). E.g., only "200" for a REST call and retry for any other status codes.
Can anyone help review this?