chia7712 commented on code in PR #875:
URL: https://github.com/apache/yunikorn-core/pull/875#discussion_r1621298030
##########
pkg/common/utils.go:
##########
@@ -224,6 +227,9 @@ func ZeroTimeInUnixNano(t time.Time) *int64 {
}
func WaitForCondition(eval func() bool, interval time.Duration, timeout
time.Duration) error {
+ if interval > timeout {
Review Comment:
Personally, changing the `interval` is a bit awkward since that breaks the
definition. Maybe we can rewrite this method.
```go
deadline := time.Now().Add(timeout)
for {
if eval() {
return nil
}
if time.Now().After(deadline) ||
time.Now().Add(interval).After(deadline) {
return fmt.Errorf("timeout waiting for condition")
}
time.Sleep(interval)
}
```
In short, we return timeout error early if it will be timeout due to
sleeping.
##########
pkg/common/utils.go:
##########
@@ -63,6 +63,9 @@ func GetPartitionNameWithoutClusterID(partitionName string)
string {
}
func WaitFor(interval time.Duration, timeout time.Duration, condition func()
bool) error {
Review Comment:
Could we keep one of `WaitForCondition` or `WaitFor`? The k8shim module has
`WaitForCondition`, and so maybe we can keep `WaitForCondition` for consistency.
https://github.com/apache/yunikorn-k8shim/blob/58adfe941d2d8dae5544af8b49e435f304678807/pkg/common/utils/utils.go#L245
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
