-
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
Switch RateLimit headers spec version to latest #12493
Conversation
Signed-off-by: Petr Pchelko <ppchelko@wikimedia.org>
@@ -29,8 +29,8 @@ message RateLimit { | |||
// X-RateLimit headers disabled. | |||
OFF = 0; | |||
|
|||
// Use `draft RFC Version 02 <https://tools.ietf.org/id/draft-polli-ratelimit-headers-02.html>`_. | |||
DRAFT_VERSION_02 = 1; |
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.
So this is just added in the previous PR, right?
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.
Yes. All the code/docs touched here were added in #12410
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.
Looks good. While the change seems OK, the API part requires @envoyproxy/api-shepherds approval.
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!
Followup for a new feature introduced by envoyproxy#12410 Apologies for not noticing that a later draft was introduced recently. I think we should start with supporting the latest available spec draft, so update version 2 to version 3. The change is technically backwards-incompatible, but the new feature was introduced one day ago, nobody could have been so fast to depend on it. Signed-off-by: Petr Pchelko <ppchelko@wikimedia.org>
Followup for a new feature introduced by envoyproxy#12410 Apologies for not noticing that a later draft was introduced recently. I think we should start with supporting the latest available spec draft, so update version 2 to version 3. The change is technically backwards-incompatible, but the new feature was introduced one day ago, nobody could have been so fast to depend on it. Signed-off-by: Petr Pchelko <ppchelko@wikimedia.org> Signed-off-by: chaoqinli <chaoqinli@google.com>
Signed-off-by: Petr Pchelko ppchelko@wikimedia.org
Followup for a new feature introduced by #12410 Apologies for not noticing that a later draft was introduced recently. I think we should start with supporting the latest available spec draft, so update version 2 to version 3. The change is technically backwards-incompatible, but the new feature was introduced one day ago, nobody could have been so fast to depend on it.
Commit Message: Switch X-RateLimit headers spec version to 3
Risk Level: Low
Testing: Existing unit/integration tests
Docs Changes: rate_limit.proto and release notes
Release Notes: changed current.rst
cc @unleashed