Skip to content

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

HappyTobi
Copy link

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

@codecov
Copy link

codecov bot commented Dec 13, 2019

Codecov Report

Merging #1433 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ Complexity Δ
...framework/cloud/gateway/webflux/ProxyExchange.java 55% <0%> (ø) 38 <0> (ø) ⬇️

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 2b8aef0...10a1d91. Read the comment docs.

@HappyTobi
Copy link
Author

Hi @spencergibb,

what can i do to get the pr merged?

I will now rebase my branch and fix the conflict again.

Kind regards
Tobi


import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.jetbrains.annotations.NotNull;

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.

Copy link
Author

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

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" :)

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.

Copy link
Author

Choose a reason for hiding this comment

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

i'll change that

@spencergibb spencergibb force-pushed the master branch 2 times, most recently from 73d934e to 5bd5791 Compare July 28, 2020 18:26
@dnandola-wyze
Copy link

Any update on why this is not being merged? Having this will be extremely useful.

@spencergibb spencergibb force-pushed the main branch 2 times, most recently from 1dc0e8a to 9efafe8 Compare November 8, 2021 21:37
@muchnik
Copy link
Contributor

muchnik commented Feb 14, 2022

@spencergibb Any plans to merge this? Looks really usefull.

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

Successfully merging this pull request may close these issues.

6 participants