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: add a component to support rate limt control #29

Merged
merged 6 commits into from
Jul 2, 2023

Conversation

PolarishT
Copy link
Contributor

@PolarishT PolarishT commented Jun 28, 2023


👀 What this PR does / why we need it:

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

🅰 Which issue(s) this PR fixes:

Fixes #20

📝 Special notes for your reviewer:

🎯 Describe how to verify it

📑 Additional documentation e.g., RFC, notion, Google docs, usage docs, etc.:

@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 28, 2023
Copy link
Contributor Author

@PolarishT PolarishT left a comment

Choose a reason for hiding this comment

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

v-0.0.1 singleton demo

@cubxxw
Copy link
Contributor

cubxxw commented Jun 28, 2023

not:

Fixes issue20

but:

Fixes #20

example: Fixes #20

@kubbot
Copy link
Contributor

kubbot commented Jun 28, 2023

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


not:

Fixed issue 20

but:

Fixes #20

example: Fixes #20

@cubxxw
Copy link
Contributor

cubxxw commented Jun 28, 2023

not:

feat: issue20 add a component to support rate limt control

but:

feat: add a component to support rate limt control

@IRONICBo
Copy link
Collaborator

Suggest to rename filename frequencyControl to frequency_control or rate_limit.

}
return func(c *gin.Context) {
if !tb.Allow() {
c.String(503, "Too many request")
Copy link
Collaborator

Choose a reason for hiding this comment

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

use constant in http or define a constant in OpenKF/server/internal/common/code.go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok i'll solve it

@PolarishT PolarishT changed the title feat: issue20 add a component to support rate limt control feat: add a component to support rate limt control Jun 28, 2023
@IRONICBo
Copy link
Collaborator

@cubxxw cubxxw temporarily deployed to openkf June 30, 2023 03:40 — with GitHub Actions Inactive
@PolarishT PolarishT enabled auto-merge June 30, 2023 03:53
@PolarishT PolarishT disabled auto-merge June 30, 2023 03:53
@PolarishT PolarishT temporarily deployed to openkf June 30, 2023 03:58 — with GitHub Actions Inactive
Copy link
Contributor Author

@PolarishT PolarishT left a comment

Choose a reason for hiding this comment

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

format coding style

@PolarishT PolarishT temporarily deployed to openkf July 2, 2023 07:36 — with GitHub Actions Inactive
Copy link
Contributor Author

@PolarishT PolarishT left a comment

Choose a reason for hiding this comment

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

fix bug

@PolarishT PolarishT self-assigned this Jul 2, 2023
@PolarishT PolarishT added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. API labels Jul 2, 2023
@PolarishT PolarishT temporarily deployed to openkf July 2, 2023 07:38 — with GitHub Actions Inactive
fix return formating

Signed-off-by: SgitBusity <[email protected]>

// allow frequency
func (tb *frequencyControlByTokenBucket) Allow() bool {
tb.mtx.Lock()
Copy link
Contributor

Choose a reason for hiding this comment

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

such as sync.RWMutex


// define rate limit struct
type frequencyControlByTokenBucket struct {
refreshRate float64 // 令牌的刷新速率
Copy link
Contributor

Choose a reason for hiding this comment

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

please use English

mtx sync.Mutex // 互斥锁
}

// allow frequency
Copy link
Contributor

Choose a reason for hiding this comment

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

func comment, capitalized

}
return func(c *gin.Context) {
if !tb.Allow() {
c.String(503, "Too many request")
Copy link
Contributor

Choose a reason for hiding this comment

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

return: gin.Context the mechanism of error processing

}

// registried a middle ware
func LimitHandler(maxConn int, refreshRate float64) gin.HandlerFunc {
Copy link
Contributor

Choose a reason for hiding this comment

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

write unit testing

// define rate limit struct
type frequencyControlByTokenBucket struct {
refreshRate float64 // 令牌的刷新速率
capacity int64 // 桶的容量
Copy link
Contributor

Choose a reason for hiding this comment

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

int?

@cubxxw cubxxw enabled auto-merge July 2, 2023 10:20
@cubxxw cubxxw disabled auto-merge July 2, 2023 10:20
@cubxxw cubxxw merged commit 66d6f35 into openimsdk:main Jul 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Add a component to support RateTime Control
4 participants