Skip to content

feat: support limit-conn in workflow plugin#12465

Merged
nic-6443 merged 14 commits into
apache:masterfrom
Revolyssup:revolyssup/limit-conn-workflow
Aug 6, 2025
Merged

feat: support limit-conn in workflow plugin#12465
nic-6443 merged 14 commits into
apache:masterfrom
Revolyssup:revolyssup/limit-conn-workflow

Conversation

@Revolyssup

@Revolyssup Revolyssup commented Jul 28, 2025

Copy link
Copy Markdown
Contributor

This PR adds support for using limit-conn plugin inside the workflow plugin extending the functionality of workflow plugin.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Jul 28, 2025
Comment thread apisix/plugins/workflow.lua
Comment thread apisix/plugins/workflow.lua Outdated
for _, rule in ipairs(conf.rules) do
local match_result = true
if rule.case then
local expr, _ = expr.new(rule.case)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

performance: we'd better to cache the expr and reuse it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can store expr in conf.rules

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of caching expr in access phase. How about doing it in check_schema?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it is acceptable

Comment thread apisix/plugins/workflow.lua Outdated
if not ok then
return false, "failed to validate the 'case' expression: " .. err
end
setmetatable(rule, {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should getmetatable first

nic-6443
nic-6443 previously approved these changes Jul 31, 2025
nic-6443
nic-6443 previously approved these changes Jul 31, 2025
Comment thread apisix/plugins/workflow.lua Outdated
if key == "__expr" then
return expr
end
local mt = getmetatable(rule) or {}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls take a look at this style:

local my_table = {}

local meta = getmetatable(my_table)
if not meta then
    meta = {}
    setmetatable(my_table, meta) 
end

meta.a = "a" 

Comment thread apisix/plugins/workflow.lua Outdated
mt = {}
setmetatable(rule, mt)
end
mt.__index = function (_, key)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this way is much better to read:

local my_table = {b="b"}

local meta = getmetatable(my_table)
if not meta then
    meta = {}
    meta.__index = meta
    setmetatable(my_table, meta)
end
meta.a = "a"

print(my_table.a)

membphis
membphis previously approved these changes Jul 31, 2025
nic-6443
nic-6443 previously approved these changes Jul 31, 2025
Comment thread apisix/plugins/workflow.lua
Comment thread apisix/plugins/workflow.lua
Comment thread apisix/plugins/workflow.lua
Comment thread apisix/plugins/limit-count.lua
@Revolyssup
Revolyssup dismissed stale reviews from nic-6443 and membphis via 8189952 August 5, 2025 10:30

@membphis membphis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nic-6443
nic-6443 merged commit ee108c5 into apache:master Aug 6, 2025
43 of 45 checks passed
@Revolyssup
Revolyssup deleted the revolyssup/limit-conn-workflow branch August 6, 2025 07:36
jizhuozhi pushed a commit to jizhuozhi/apisix that referenced this pull request Oct 18, 2025
shreemaan-abhishek pushed a commit to shreemaan-abhishek/apisix that referenced this pull request Jan 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants