Skip to content

fix for issue #988 --> seemles highly customizable retry setup #989

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MarkusSchildhauer
Copy link

@MarkusSchildhauer MarkusSchildhauer commented Jan 11, 2023

Solves issue #988 to my understanding.

features:

  • retry_check is always executed/checked if retryable(req) returns true.
  • retry_check keyword arguments defaults to function HTTP.retry_check respectively HTTP.isrecoverable which restores the old behaviour that minimizes unnecessary (little promising) retries.
  • retry_delays is not restricted to ExponentialBackOff, it can be any iterator with length over Reals
  • retry defaults to retries>0
  • retries is default n for ExponentialBackOff, but is overwritten by length(retry_delays) to avoid inconsistencies caused by req.context[:retrylimitreached]
  • even though I don't see the use of req.context[:retrylimitreached] and req.context[:retryattempt] it's consistently integrated into the Base.retry-check-function.

@codecov-commenter
Copy link

codecov-commenter commented Jan 11, 2023

Codecov Report

Merging #989 (393b7b2) into master (fe9bf99) will increase coverage by 0.11%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master     #989      +/-   ##
==========================================
+ Coverage   82.53%   82.64%   +0.11%     
==========================================
  Files          37       37              
  Lines        3040     3043       +3     
==========================================
+ Hits         2509     2515       +6     
+ Misses        531      528       -3     
Impacted Files Coverage Δ
src/clientlayers/RetryRequest.jl 77.27% <80.00%> (+6.54%) ⬆️
src/Messages.jl 85.14% <0.00%> (+0.57%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants