-
Notifications
You must be signed in to change notification settings - Fork 339
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
docs: added MADR for MeshRateLimit #5333
Conversation
Signed-off-by: Łukasz Dziedziak <lukidzi@gmail.com>
Signed-off-by: Łukasz Dziedziak <lukidzi@gmail.com>
Signed-off-by: Łukasz Dziedziak <lukidzi@gmail.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.
Great start! I'd like us anticipate potentially having a global rate limit in this policy in the future.
Might be worth to have something like:
default:
local:
http: ...
Also it might be useful to have a disabled
flag to be potentially used when overriding defaults in child policy. I'm not sure how we'd do this other @lobkovilya interested in your opinion here.
Please reference the issue in this PR! |
Signed-off-by: Łukasz Dziedziak <lukidzi@gmail.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.
SOme minor comments but looking good!
Co-authored-by: Charly Molter <charly@molter.io> Signed-off-by: Łukasz Dziedziak <lukidzi@gmail.com>
Co-authored-by: Charly Molter <charly@molter.io> Signed-off-by: Łukasz Dziedziak <lukidzi@gmail.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.
LGTM!
Signed-off-by: Łukasz Dziedziak <lukidzi@gmail.com>
syscall.Mkfifo
have equivalent implementation on the other OS --UPGRADE.md
? --> Changelog:
entry here or add aci/
label to run fewer/more tests?