Refactor WaitFor (#221)
* Refactor WaitFor
This commit modifies WaitFor in a few ways:
It replaces time.Now().Second() with time.Now().Unix() in order to
account for rolling minutes (for example, when WaitFor starts at 59
seconds, every timeout check will result in a negative number).
A "retry" timer has also been added. This will cause the predicate
to be retried every n seconds if there hasn't been a response. This
is to account for server or network issues that would cause the
predicate to be lost or hang indefinitely.
A combination of using both timeout and retry can be effective in
handling faulty requests as well as a master kill switch to stop.
* Remove Retry from WaitFor
This removes the retry parameter and re-uses the timeout to kill
possible hanging predicates.
diff --git a/testing/util_test.go b/testing/util_test.go
index 5985bc3..ae3e448 100644
--- a/testing/util_test.go
+++ b/testing/util_test.go
@@ -1,22 +1,58 @@
package testing
import (
+ "errors"
"os"
"path/filepath"
"strings"
"testing"
+ "time"
"github.com/gophercloud/gophercloud"
th "github.com/gophercloud/gophercloud/testhelper"
)
func TestWaitFor(t *testing.T) {
- err := gophercloud.WaitFor(5, func() (bool, error) {
+ err := gophercloud.WaitFor(2, func() (bool, error) {
return true, nil
})
th.CheckNoErr(t, err)
}
+func TestWaitForTimeout(t *testing.T) {
+ if testing.Short() {
+ t.Skip("skipping test in short mode.")
+ }
+
+ err := gophercloud.WaitFor(1, func() (bool, error) {
+ return false, nil
+ })
+ th.AssertEquals(t, "A timeout occurred", err.Error())
+}
+
+func TestWaitForError(t *testing.T) {
+ if testing.Short() {
+ t.Skip("skipping test in short mode.")
+ }
+
+ err := gophercloud.WaitFor(2, func() (bool, error) {
+ return false, errors.New("Error has occurred")
+ })
+ th.AssertEquals(t, "Error has occurred", err.Error())
+}
+
+func TestWaitForPredicateExceed(t *testing.T) {
+ if testing.Short() {
+ t.Skip("skipping test in short mode.")
+ }
+
+ err := gophercloud.WaitFor(1, func() (bool, error) {
+ time.Sleep(4 * time.Second)
+ return false, errors.New("Just wasting time")
+ })
+ th.AssertEquals(t, "A timeout occurred", err.Error())
+}
+
func TestNormalizeURL(t *testing.T) {
urls := []string{
"NoSlashAtEnd",
diff --git a/util.go b/util.go
index 3d6a4e4..68f9a5d 100644
--- a/util.go
+++ b/util.go
@@ -1,7 +1,7 @@
package gophercloud
import (
- "errors"
+ "fmt"
"net/url"
"path/filepath"
"strings"
@@ -9,27 +9,47 @@
)
// WaitFor polls a predicate function, once per second, up to a timeout limit.
-// It usually does this to wait for a resource to transition to a certain state.
+// This is useful to wait for a resource to transition to a certain state.
+// To handle situations when the predicate might hang indefinitely, the
+// predicate will be prematurely cancelled after the timeout.
// Resource packages will wrap this in a more convenient function that's
// specific to a certain resource, but it can also be useful on its own.
func WaitFor(timeout int, predicate func() (bool, error)) error {
- start := time.Now().Second()
+ type WaitForResult struct {
+ Success bool
+ Error error
+ }
+
+ start := time.Now().Unix()
+
for {
- // Force a 1s sleep
+ // If a timeout is set, and that's been exceeded, shut it down.
+ if timeout >= 0 && time.Now().Unix()-start >= int64(timeout) {
+ return fmt.Errorf("A timeout occurred")
+ }
+
time.Sleep(1 * time.Second)
- // If a timeout is set, and that's been exceeded, shut it down
- if timeout >= 0 && time.Now().Second()-start >= timeout {
- return errors.New("A timeout occurred")
- }
+ var result WaitForResult
+ ch := make(chan bool, 1)
+ go func() {
+ defer close(ch)
+ satisfied, err := predicate()
+ result.Success = satisfied
+ result.Error = err
+ }()
- // Execute the function
- satisfied, err := predicate()
- if err != nil {
- return err
- }
- if satisfied {
- return nil
+ select {
+ case <-ch:
+ if result.Error != nil {
+ return result.Error
+ }
+ if result.Success {
+ return nil
+ }
+ // If the predicate has not finished by the timeout, cancel it.
+ case <-time.After(time.Duration(timeout) * time.Second):
+ return fmt.Errorf("A timeout occurred")
}
}
}