-
Notifications
You must be signed in to change notification settings - Fork 3.4k
QuotaFilter for Spring-Cloud-Gateway #1433
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
base: main
Are you sure you want to change the base?
Conversation
d9e932a
to
a24a81c
Compare
Codecov Report
@@ Coverage Diff @@
## master #1433 +/- ##
=========================================
Coverage 63.32% 63.32%
Complexity 92 92
=========================================
Files 8 8
Lines 439 439
Branches 34 34
=========================================
Hits 278 278
Misses 140 140
Partials 21 21
Continue to review full report at Codecov.
|
...org/springframework/cloud/gateway/filter/factory/RequestRateLimiterGatewayFilterFactory.java
Outdated
Show resolved
Hide resolved
Hi @spencergibb, what can i do to get the pr merged? I will now rebase my branch and fix the conflict again. Kind regards |
|
||
import org.apache.commons.logging.Log; | ||
import org.apache.commons.logging.LogFactory; | ||
import org.jetbrains.annotations.NotNull; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is a mistake import.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its the same that the RedisRateLimiter use, but yes think it would be better to switch to slf4j
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I talk about "org.jetbrains.annotations" :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed there is the same in RedisRateLimiter I think it was also a mistake in RedisRateLimiter and "javax.validation.constraints.NotNull" should be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'll change that
73d934e
to
5bd5791
Compare
Any update on why this is not being merged? Having this will be extremely useful. |
1dc0e8a
to
9efafe8
Compare
@spencergibb Any plans to merge this? Looks really usefull. |
Hi all,
there is the PR for the issue #1402
The Quota Filter needs Redis like the RateLimiter, no new dependencies needed for the quota filter.
With the filter its super easy to create quotas bases on key-resolvers for the services that are running behind the Gateway.
I also added the documentation about the usage of the filter.
Hope you like it.
Kind regards
Tobi