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: support timeout and failOpen/failClose knobs for Global RateLimiting #1704

Merged
merged 8 commits into from
Aug 3, 2023

Conversation

tmsnan
Copy link
Contributor

@tmsnan tmsnan commented Jul 25, 2023

Support timeout and failOpen/failClose knobs for Global RateLimiting
releated #1655

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>
@tmsnan tmsnan requested a review from a team as a code owner July 25, 2023 06:33
Signed-off-by: zhaonan <zhaonan06@corp.netease.com>
@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #1704 (5db58ef) into main (72bfe9b) will decrease coverage by 0.05%.
The diff coverage is 23.07%.

@@            Coverage Diff             @@
##             main    #1704      +/-   ##
==========================================
- Coverage   64.90%   64.85%   -0.05%     
==========================================
  Files          84       84              
  Lines       12014    12023       +9     
==========================================
  Hits         7798     7798              
- Misses       3727     3734       +7     
- Partials      489      491       +2     
Files Changed Coverage Δ
internal/xds/translator/runner/runner.go 59.25% <0.00%> (-3.49%) ⬇️
internal/xds/translator/translator.go 78.44% <ø> (ø)
internal/xds/translator/ratelimit.go 87.13% <33.33%> (-1.72%) ⬇️

@tmsnan tmsnan force-pushed the dev-extend-global-ratelimit branch from 4e46cb2 to 4185459 Compare July 25, 2023 06:37
Signed-off-by: zhaonan <zhaonan06@corp.netease.com>
@arkodg
Copy link
Contributor

arkodg commented Jul 25, 2023

some comments in API naming and types, but looks good overall, thanks !

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>
@tmsnan
Copy link
Contributor Author

tmsnan commented Jul 26, 2023

/retest

Signed-off-by: zhaonan <zhaonan06@corp.netease.com>
@tmsnan
Copy link
Contributor Author

tmsnan commented Jul 26, 2023

/retest

Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM thanks !

@arkodg arkodg requested review from a team, kflynn, zhaohuabing and chauhanshubham and removed request for a team July 26, 2023 18:35
Copy link
Member

@Xunzhuo Xunzhuo left a comment

Choose a reason for hiding this comment

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

/lgtm, thanks!

@Xunzhuo Xunzhuo changed the title feat: Support timeout and failOpen/failClose knobs for Global RateLimiting feat: support timeout and failOpen/failClose knobs for Global RateLimiting Jul 27, 2023
@zirain
Copy link
Contributor

zirain commented Jul 27, 2023

cc @arkodg, should this landing in v0.5?

@arkodg
Copy link
Contributor

arkodg commented Jul 27, 2023

@zirain since this is an feature/API change, and rc has been released for v0.5, this cannot make it in.
Lets merge this PR once v0.5 release, since this is an API change and affects docs dir which is yet to be created for v0.5

@arkodg arkodg added this to the 0.6.0-rc1 milestone Jul 27, 2023
@arkodg arkodg merged commit 7eb3cae into envoyproxy:main Aug 3, 2023
17 of 18 checks passed
@tmsnan tmsnan deleted the dev-extend-global-ratelimit branch September 6, 2023 12:30
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.

None yet

4 participants