nastra commented on PR #8715: URL: https://github.com/apache/iceberg/pull/8715#issuecomment-1748190639
The goal of https://github.com/apache/iceberg/issues/7154 is to convert `Thread.sleep` usages to Awaitility where it makes sense. We don't want to blindly just replace all `Thread.sleep` usage. We need to understand why the sleep is used in the first place. Typically, `Thread.sleep` is used to wait for an async condition to happen. Rather than waiting the max sleep time, we want to wait less than that, by checking a success condition that indicates we can proceed. Here is a good example where the test was made more stable by the use of Awaitility: https://github.com/apache/iceberg/blob/cff2ff9d2f533c8e733324a66f3101eafbc2e932/core/src/test/java/org/apache/iceberg/actions/TestCommitService.java#L99-L102 We wait at most 5 seconds until the given assertion is met. For this PR that means a good starting point might be https://github.com/apache/iceberg/blob/f9f85e7f2647020ef7e9571f76027c8e55075690/flink/v1.17/flink/src/test/java/org/apache/iceberg/flink/source/TestStreamingMonitorFunction.java#L153, where there's a 1s sleep and then a check. That's the kind of things we want to replace with Awaitility -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
