Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: add support for multiple retries #5

Merged
merged 9 commits into from
Feb 9, 2020

Conversation

ayushya
Copy link
Collaborator

@ayushya ayushya commented Jan 27, 2020

Summary
I have added support for multiple retries via a config parameter maxRetries whose default value is 1.

This PR has the following changes:

  1. Added support for multiple retries via maxRetries option.
  2. Default value of maxRetries is 1.
  3. Since each of the retry attempts should have a unique URL, the cacheBusting URL is changed on each retry attempt. This is done by calling the functiongetCacheBustString every time.
  4. Have updated the README with the instructions to use this option.

Update

  1. Have updated the e2e to testing suite to verify the behavior with maxRetries.
  2. We also append &retryAttempt=attemptValue in the query string to track retryAttempt.

This closes #4

@codecov-io
Copy link

codecov-io commented Jan 27, 2020

Codecov Report

Merging #5 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master     #5   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines          13     16    +3     
  Branches        2      3    +1     
=====================================
+ Hits           13     16    +3
Impacted Files Coverage Δ
src/index.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3bb6922...90058fa. Read the comment docs.

src/index.js Outdated Show resolved Hide resolved
@ayushya
Copy link
Collaborator Author

ayushya commented Jan 30, 2020

@mattlewis92 Did you get a chance to review this PR?

src/index.js Outdated Show resolved Hide resolved
src/index.js Show resolved Hide resolved
@ayushya
Copy link
Collaborator Author

ayushya commented Feb 7, 2020

@b1rdex I have updated the PR.
Can you re-review this?

Copy link
Owner

@mattlewis92 mattlewis92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great work @ayushya and thanks a lot to @b1rdex for reviewing as well. Really appreciate it! ❤️

@mattlewis92 mattlewis92 changed the title Support for Multiple Retries feat: add support for multiple retries Feb 9, 2020
@mattlewis92 mattlewis92 merged commit 04af01d into mattlewis92:master Feb 9, 2020
@mattlewis92
Copy link
Owner

Released as 1.2.0 🎉

@ayushya
Copy link
Collaborator Author

ayushya commented Feb 9, 2020

This is great work @ayushya and thanks a lot to @b1rdex for reviewing as well. Really appreciate it! ❤️

It was fun to work on this.

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

Successfully merging this pull request may close these issues.

Support for custom/multiple retries via params
4 participants