Baoyuantop commented on issue #13179: URL: https://github.com/apache/apisix/issues/13179#issuecomment-4248719841
@niemne Thanks for the detailed proposal! The overall direction looks good and references the right PRs. A few adjustments are needed though: 1. **Remove `header_prefix`**: Unlike `limit-count` which sets `X-RateLimit-*` response headers, `limit-req` does not set any rate-limiting response headers currently. So `header_prefix` has no practical effect here. If we want to add response headers for `limit-req` in the future, it should be a separate enhancement. 2. **Remove `key_type` from rules**: Following the pattern of `limit-conn` and `limit-count` rules implementation (see `apisix/plugins/limit-conn/init.lua:64-98`), the `key` in rules is always resolved as a variable combination via `core.utils.resolve_var`. If no variables are resolved (`n_resolved == 0`), the rule is skipped. There's no need for a `key_type` field in rules. The rest of the proposal looks good: - `oneOf` variable support for `rate`/`burst` - `oneOf` schema ensuring legacy and rules modes are mutually exclusive - Removing `lrucache.plugin_ctx` in favor of per-request limiter creation Please update the proposal with the above adjustments and start the implementation. I'd also suggest extracting the core logic into `apisix/plugins/limit-req/init.lua` to keep the directory structure consistent with `limit-conn`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
