-
Notifications
You must be signed in to change notification settings - Fork 1
feat(ipa0114): Add rate limiting guidelines to errors IPA #27
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
5758947 to
7b2b238
Compare
ipa/general/0114.md
Outdated
| - APIs **must** include the `Retry-After` HTTP response header when returning | ||
| `429 Too Many Requests` to indicate when the client can retry the request. | ||
| - The `Retry-After` header value **must** be expressed as time in seconds | ||
| until the next token is available. |
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.
| - APIs **must** include the `Retry-After` HTTP response header when returning | |
| `429 Too Many Requests` to indicate when the client can retry the request. | |
| - The `Retry-After` header value **must** be expressed as time in seconds | |
| until the next token is available. | |
| - APIs **should** include the `Retry-After` HTTP response header when returning | |
| `429 Too Many Requests` to indicate how long the client should wait before retrying the request. | |
| - The `Retry-After` header value **must** be expressed as time in seconds | |
| until the next request can be made. |
I think the Retry-After should be a guideline and not necessarily a must, similarly to the rate limit headers
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.
SGTM
ipa/general/0114.md
Outdated
| - `RateLimit-Limit`: The maximum number of requests allowed in the current | ||
| rate limit window (bucket capacity) | ||
| - `RateLimit-Remaining`: The number of requests remaining in the current rate | ||
| limit window (remaining tokens) |
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.
Q: Should we use token bucket specific language? Or refer to maximum requests and remaining requests?
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.
Good point, I think we actually shouldn't use token bucket specific language since there could be a different implementation and the headers aren't specific to token bucket
lovisaberggren
left a comment
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.
Ty!
Ticket: CLOUDP-366822
Add rate limiting guidelines to errors IPA
Note: Will be merged as a post-rollout action