Add conditions to rate-limit policy #839
Conversation
| }, | ||
| "operation": { | ||
| "type": "object", | ||
| "$id": "#/definitions/operation", |
There was a problem hiding this comment.
Can we define those in the policy manifest spec? And then just refer to those?
There was a problem hiding this comment.
You mean manifest-schema.json ?
There was a problem hiding this comment.
Yes. It could make the validation harder, but sharing those definitions would be good in my opinion.
There was a problem hiding this comment.
Yes, it would avoid duplication, but I'm not sure if the tooling we have for validation will work correctly. I'll try it.
There was a problem hiding this comment.
Actually, I think it's better to not include them in manifest-schema.json. I'd rather duplicate that for now in the 2 policies that need those definitions than opening them in the schema and having to support them in case someones starts depending on them. At least until the conditional policy is more mature.
t/apicast-policy-rate-limit.t
Outdated
| lua_shared_dict limiter 1m; | ||
| --- config | ||
| include $TEST_NGINX_APICAST_CONFIG; | ||
| resolver $TEST_NGINX_RESOLVER; |
There was a problem hiding this comment.
Right, copy-pasted that before I saw your new PR :D
davidor
force-pushed
the
rate-limit-conditions
branch
from
4ded891
to
27312ff
Compare
Closes the last item of the TODO list in #760