[GitHub] [flink] gaoyunhaii commented on pull request #21077: [FLINK-29498] Add Scala Async Retry Strategies and ResultPredicates Helper Classes

[GitBox <gi...@apache.org>]

gaoyunhaii commented on PR #21077:
URL: https://github.com/apache/flink/pull/21077#issuecomment-1298319905

   Hi @ericxiao251 very thanks for the update! I think we might be able to simplify the wrapper a bit with:
   1. for `RetryPredicates#EMPTY_RESULT_PREDICATE` and `RetryPredicates#HAS_EXCEPTION_PREDICATE`, it seems we do not need to wrap it since both the Scala version and the java version extends the same interface, namely `java.util.function.Predicate`. We might directly import the name into the Scala packages via
   
   ```
   val EMPTY_RESULT_PREDICATE = JRetryPredicates.EMPTY_RESULT_PREDICATE;
   val HAS_EXCEPTION_PREDICATE = JRetryPredicates.HAS_EXCEPTION_PREDICATE;
   ```
   
   2. For AsyncRetryPredicate, we might have a uniform wrapper for Java version of AsyncRetryPredicate. Then we do not need to have Scala version of `FixedDelayRetryStrategy` and  `ExponentialBackoffDelayRetryStrategy`.  Instead we could make `FixedDelayRetryStrategyBuilder` and `ExponentialBackoffDelayRetryStrategyBuilder` returns the wrapped strategy directly like
   
   ```
   def build(): AsyncRetryPredicate[OUT] = {
           return new JavaAsyncRetryStrategyWrapper(new 
             JAsyncRetryStrategies.ExponentialBackoffDelayRetryStrategyBuilder[OUT](
             maxAttempts,
             lastRetryDelay,
             maxRetryDelay,
             multiplier).build());
   }
   ```
   
   How do you think about that? And very sorry if I caused misleadings initially. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org