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

refactor: rate limiter and throttle middleware #518

Merged
merged 11 commits into from
Jun 27, 2024
Merged

Conversation

devhaozi
Copy link
Member

@devhaozi devhaozi commented Jun 25, 2024

πŸ“‘ Description

Closes goravel/goravel#448

https://github.com/sethvargo/go-limiter package's go.mod configure to go 1.22, so it cannot be merged into v1.14.x.

βœ… Checks

  • Added test cases for my code

Copy link

codecov bot commented Jun 25, 2024

Codecov Report

Attention: Patch coverage is 73.62637% with 24 lines in your changes missing coverage. Please review.

Project coverage is 69.90%. Comparing base (00f7e7b) to head (6246b8c).

Files Patch % Lines
http/limit/store.go 74.62% 13 Missing and 4 partials ⚠️
http/limit/limit.go 0.00% 4 Missing ⚠️
http/middleware/throttle.go 85.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #518      +/-   ##
==========================================
- Coverage   69.97%   69.90%   -0.07%     
==========================================
  Files         179      180       +1     
  Lines       10952    11000      +48     
==========================================
+ Hits         7664     7690      +26     
- Misses       2719     2739      +20     
- Partials      569      571       +2     

β˜” View full report in Codecov by Sentry.
πŸ“’ Have feedback on the report? Share it here.

Copy link
Contributor

@hwbrzzl hwbrzzl left a comment

Choose a reason for hiding this comment

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

Great PR, just a few questions.

@hwbrzzl
Copy link
Contributor

hwbrzzl commented Jun 25, 2024

And please add some integration tests in goravel/example after merging this.

@devhaozi devhaozi requested review from hwbrzzl, kkumar-gcc and a team June 25, 2024 18:00
@devhaozi devhaozi marked this pull request as draft June 25, 2024 18:05
@devhaozi devhaozi marked this pull request as ready for review June 25, 2024 18:05
@devhaozi devhaozi self-assigned this Jun 25, 2024
@devhaozi devhaozi enabled auto-merge (squash) June 25, 2024 18:27
Copy link
Contributor

@hwbrzzl hwbrzzl left a comment

Choose a reason for hiding this comment

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

Great optimization

@devhaozi devhaozi requested a review from hwbrzzl June 26, 2024 07:02
@devhaozi devhaozi requested a review from hwbrzzl June 27, 2024 06:50
Copy link
Contributor

@hwbrzzl hwbrzzl left a comment

Choose a reason for hiding this comment

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

Great πŸ‘

@devhaozi devhaozi merged commit 0ad5442 into master Jun 27, 2024
9 of 10 checks passed
@devhaozi devhaozi deleted the haozi/limiter branch June 27, 2024 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

πŸ› [Bug] 倚δΈͺι™ζ΅ε™¨ζ—Άηš„Bug
2 participants