Good catch, thanks.

I would remove the argument in seconds and let the caller pass a context with a deadline. The predicate function should also accept a context, so that we delegate to the caller the cancellation of hanging functions. The function could be the just called “wait”. WDYT?

@pierreprinetti indeed in many cases context with the deadline is enough, but i found it handy option to pass a timeout.

Context of the predicate anyway should be canceled once we exit WaitFor, so it's just handy way of use either WithCancel or WithTimeout if timeout > 0.

I'm unsure about other WaitFor* funcs, unfortunately my regex-fu is not that great...

i found it handy option to pass a timeout

The context library provides a convenience function for setting a timeout. Note that if you set a deadline or timeout on a context that already has one, the shortest delay correctly wins.

My stance here is: this is not Gophercloud's job. There are plenty of libraries out there that do Wait in fanciful ways. I understand that we might continue providing a function that covers the basic case, but we should keep complexity at a minimum to avoid bugs. Accepting a context and letting the user set it appropriately seems to me like a reasonable compromise.

@pierreprinetti agree, that surface should be minimal but enough. I also agree, that usually you just pass deadline ctx and do not expect fancy logic inside.

Optional timeout just makes tests a bit easier to write, but i often have limited context in test suite anyway.

I'll drop timeouts.

@pierreprinetti done.

@vooon we may have issues here: internal/acceptance/openstack/containerinfra/v1/nodegroups_test.go

https://github.com/gophercloud/gophercloud/actions/runs/8237438492/job/22526174282?pr=2957#step:5:969

@pierreprinetti thanks, fixed.

Nice! Thank you @vooon and @mandre for the additional review. I think we can merge as soon as we get the CI's blessing