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.

Sign up for GitHub

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

Jump to bottom

Added exponential backoff #387

Merged
merged 10 commits into from
Nov 15, 2024
Merged

Added exponential backoff #387

merged 10 commits into from
Nov 15, 2024

Conversation

aniketcodes
Copy link
Contributor

This PR adds the capability to add exponential backoff.
#279

Checklist

@aniketcodes aniketcodes mentioned this pull request Oct 29, 2024
Closed
2 tasks
Uzlopak
Uzlopak previously requested changes Nov 4, 2024
View reviewed changes
Copy link
Contributor

@Uzlopak Uzlopak left a comment

Choose a reason for hiding this comment

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

I dont like it somehow. Blocking it so that i can think about it in a free minute.

gurgunday
gurgunday reviewed Nov 4, 2024
View reviewed changes
store/LocalStore.js Outdated Show resolved Hide resolved
@aniketcodes @gurgunday
Update store/LocalStore.js
cb5940a 
Co-authored-by: Gürgün Dayıoğlu <[email protected]>
Signed-off-by: Aniket Sinha <[email protected]>
gurgunday
gurgunday reviewed Nov 4, 2024
View reviewed changes
store/RedisStore.js Outdated Show resolved Hide resolved
@aniketcodes @gurgunday
Update store/RedisStore.js
4c57264 
Co-authored-by: Gürgün Dayıoğlu <[email protected]>
Signed-off-by: Aniket Sinha <[email protected]>
@aniketcodes aniketcodes requested a review from gurgunday November 4, 2024 10:23
gurgunday
gurgunday reviewed Nov 4, 2024
View reviewed changes
Copy link
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

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

LGTM other than a final nit

store/LocalStore.js Outdated Show resolved Hide resolved
@aniketcodes aniketcodes requested a review from gurgunday November 4, 2024 13:59
@aniketcodes
Copy link
Contributor Author

Hi @gurgunday , @Uzlopak I have updated the lua script to make the ttl resetting consistent incase of continueExceeding and exponentialBackoff

gurgunday
gurgunday approved these changes Nov 6, 2024
View reviewed changes
Copy link
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

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

lgtm, @Uzlopak might have some other ideas to implement this though

@aniketcodes
Copy link
Contributor Author

@Uzlopak Can you suggest a better approach for this?

@gurgunday gurgunday requested review from a team and climba03003 November 11, 2024 09:23
@Uzlopak
Copy link
Contributor

Uzlopak commented Nov 11, 2024

Will review it today :)

Fdawgs
Fdawgs requested changes Nov 12, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
@aniketcodes @Fdawgs
Update README.md
9d84a4c 
Co-authored-by: Frazer Smith <[email protected]>
Signed-off-by: Aniket Sinha <[email protected]>
@aniketcodes aniketcodes requested a review from Fdawgs November 12, 2024 13:33
@aniketcodes
Copy link
Contributor Author

@Uzlopak @climba03003 @Fdawgs Guys, can you please review?

gurgunday
gurgunday reviewed Nov 15, 2024
View reviewed changes
store/RedisStore.js Outdated Show resolved Hide resolved
@Uzlopak Uzlopak dismissed their stale review November 15, 2024 09:20

I trust in this gurgunday

Fdawgs
Fdawgs approved these changes Nov 15, 2024
View reviewed changes
Copy link
Member

@Fdawgs Fdawgs left a comment

Choose a reason for hiding this comment

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

Docs wise it lgtm

@aniketcodes @gurgunday
Update store/RedisStore.js
0ac9a5d 
Co-authored-by: Gürgün Dayıoğlu <[email protected]>
Signed-off-by: Aniket Sinha <[email protected]>
@aniketcodes
Copy link
Contributor Author

Done @gurgunday

gurgunday
gurgunday requested changes Nov 15, 2024
View reviewed changes
types/index.d.ts Show resolved Hide resolved
store/RedisStore.js Outdated Show resolved Hide resolved
@gurgunday gurgunday merged commit 93ad9b2 into fastify:master Nov 15, 2024
10 checks passed
@gurgunday
Copy link
Member

Thanks for the PR and taking the time to address the feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gurgunday gurgunday gurgunday approved these changes

@Fdawgs Fdawgs Fdawgs approved these changes

@Uzlopak Uzlopak Awaiting requested review from Uzlopak

@climba03003 climba03003 Awaiting requested review from climba03003

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aniketcodes @Uzlopak @gurgunday @Fdawgs