octocrab
octocrab copied to clipboard
Add retry delays
Factors in two possible ways Github can indicate a retry delay. But then falls back to an exponentially increasing value. But in both cases, will cap the delay.
Now that the repo is using tower
, this PR can use tower::retry
to add the retry with exponential backoff
Now that the repo is using
tower
, this PR can usetower::retry
to add the retry with exponential backoff
For reference, octocrab already use tower::retry
https://github.com/XAMPPRocky/octocrab/blob/main/src/service/middleware/retry.rs, but it has a very simple logic (3 retries), so maybe the changes should go there to implement exponential backoff
but it has a very simple logic (3 retries), so maybe the changes should go there to implement exponential backoff
Yeah let's change that that type to use this flow. If @goodspark isn't available or doesn't have the time to change it, someone else is welcome to make as long as they include them as a co-author in the commit.
I can give it a whirl
This is gonna be a bit tricky with the current tower retry traits. I think I'd recommend waiting until https://github.com/tower-rs/tower/issues/682 lands so we can just leverage the changes to the retry policy trait and the backoff.
Key complication is that I need to chain two futures together (the future from calling sleep and the future for returning the next policy). And the typing for that is turning out to be a nightmare.
Actually, we could possibly just ref the commit in the dependency to tower. Let me try that.
Pushed a change that uses the new retry policy stuff. Basically lets us mutate the policy object instead of returning a new one. This simplifies the returned future type to just the tokio::time::Sleep future, since we'll just mutate the RetryConfig's fields.
But this doesn't compile. I think it's bc Rust is getting confused about which version of tower-layer is being used (since I'm telling it pull down tower from a repo+commit, which includes tower-layer). I'm not sure how to fix that, though.