-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
rate_limit: Add time unit month and year #24070
Conversation
Signed-off-by: tyxia <tyxia@google.com>
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
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.
Thanks for working on this!
Please add release notes as this is a user-facing change.
Signed-off-by: tyxia <tyxia@google.com>
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.
Thanks for the review! Release Note has been added.
PTAL
Signed-off-by: tyxia <tyxia@google.com>
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.
Thanks!
Code changes LGTM!
/lgtm api
cc'ing @esmet @mattklein123 as filter codeowners |
Following on from #24070 and adding support for week as a unit of time for ratelimits as this is something my team currently needs and WEEK felt sad as it was missing the protobuf party. Ideally we would have something like #33277 but in the interim it makes sense to add WEEK here since we have every other period already except week. Signed-off-by: Stefan Sedich <stefan.sedich@gmail.com>
Following on from envoyproxy/envoy#24070 and adding support for week as a unit of time for ratelimits as this is something my team currently needs and WEEK felt sad as it was missing the protobuf party. Ideally we would have something like envoyproxy/envoy#33277 but in the interim it makes sense to add WEEK here since we have every other period already except week. Signed-off-by: Stefan Sedich <stefan.sedich@gmail.com> Mirrored from https://github.com/envoyproxy/envoy @ 33fb49977a10657a84a4aff1ea8c7988c54e3c42
Per #23844 and #24031, @renuka-fernando wants to have time unit
month
andyear
for his business needRelease note added.
Signed-off-by: Tianyu Xia tyxia@google.com