add Wait and WaitCtx convenience functions #1
Conversation
|
CC @RahulAggarwal1016 |
| @@ -137,6 +139,39 @@ func (tb *TokenBucket) TryToFulfill(amount Tokens) (fulfilled bool, tryAgainAfte | |||
| return true, 0 | |||
| } | |||
|
|
|||
| // Wait removes the given amount, waiting as long as necessary. | |||
| func (tb *TokenBucket) Wait(amount Tokens) { | |||
There was a problem hiding this comment.
Is Wait equivalent to WaitCtx(context.Background(), amount)? Is it worth expressing it like that, or are you concerned about the added cost of the context accesses?
There was a problem hiding this comment.
Good point, I removed the no-context version.
There was a problem hiding this comment.
Oh, I wasn't saying to remove it, I was just suggesting that you could implement Wait as WaitCtx(context.Background(), amount). But this works too.
There was a problem hiding this comment.
I was thinking callers can just pass context.Background(). I realized that it is annoying that it returns an error in that case though, added it back.
token_bucket_test.go
Outdated
| ctxCancel() | ||
| select { | ||
| case err := <-waitResult: | ||
| if err == nil || !strings.Contains(err.Error(), "context canceled") { |
There was a problem hiding this comment.
nit: can we replace the string comparison with errors.Is(err, context.Canceled)?
The common usage for `TryToFulfill` is to sleep in a loop. This code adds a convenience `Wait` method and a corresponding `WaitCtx` which respects context cancelation.
|
TFTR! |
The common usage for
TryToFulfillis to sleep in a loop. This code adds a convenienceWaitmethod and a correspondingWaitCtxwhich respects context cancelation.