You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/10/29 12:39:13 UTC
[GitHub] [apisix] starsz opened a new pull request #2566: chore: move try_read_attr function into table.lua (#2257)
starsz opened a new pull request #2566:
URL: https://github.com/apache/apisix/pull/2566
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
https://github.com/apache/apisix/issues/2557
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [ ] Is this PR backward compatible?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] starsz commented on a change in pull request #2566: chore: move try_read_attr function into table.lua (#2257)
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #2566:
URL: https://github.com/apache/apisix/pull/2566#discussion_r514368879
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -29,6 +28,7 @@ local core = require("apisix.core")
local hmac = require("resty.hmac")
local consumer = require("apisix.consumer")
local ngx_decode_base64 = ngx.decode_base64
+local try_read_attr = core.table.try_read_attr
Review comment:
OK. Let me fix those points.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2566: chore: move try_read_attr function into table.lua (#2257)
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2566:
URL: https://github.com/apache/apisix/pull/2566#discussion_r514646591
##########
File path: apisix/plugins/log-rotate.lua
##########
@@ -152,25 +150,11 @@ local function scan_log_folder()
end
-local function try_attr(t, ...)
- local count = select('#', ...)
- for i = 1, count do
- local attr = select(i, ...)
- t = t[attr]
- if type(t) ~= "table" then
- return false
- end
- end
-
- return true
-end
-
-
local function rotate()
local local_conf = core.config.local_conf()
local interval = INTERVAL
local max_kept = MAX_KEPT
- if try_attr(local_conf, "plugin_attr", "log-rotate") then
+ if core.table.try_read_attr(local_conf, "plugin_attr", "log-rotate") then
local attr = local_conf.plugin_attr["log-rotate"]
Review comment:
Ditto
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -328,7 +312,7 @@ local function get_params(ctx)
local date_key = DATE_KEY
local signed_headers_key = SIGNED_HEADERS_KEY
- if try_attr(local_conf, "plugin_attr", "hmac-auth") then
+ if core.table.try_read_attr(local_conf, "plugin_attr", "hmac-auth") then
local attr = local_conf.plugin_attr["hmac-auth"]
Review comment:
`try_read_attr` already gets what we need, we don't need to read it again.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] starsz commented on a change in pull request #2566: chore: move try_read_attr function into table.lua (#2257)
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #2566:
URL: https://github.com/apache/apisix/pull/2566#discussion_r514788575
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -328,7 +312,7 @@ local function get_params(ctx)
local date_key = DATE_KEY
local signed_headers_key = SIGNED_HEADERS_KEY
- if try_attr(local_conf, "plugin_attr", "hmac-auth") then
+ if core.table.try_read_attr(local_conf, "plugin_attr", "hmac-auth") then
local attr = local_conf.plugin_attr["hmac-auth"]
Review comment:
OK. I had fixed.
##########
File path: apisix/plugins/log-rotate.lua
##########
@@ -152,25 +150,11 @@ local function scan_log_folder()
end
-local function try_attr(t, ...)
- local count = select('#', ...)
- for i = 1, count do
- local attr = select(i, ...)
- t = t[attr]
- if type(t) ~= "table" then
- return false
- end
- end
-
- return true
-end
-
-
local function rotate()
local local_conf = core.config.local_conf()
local interval = INTERVAL
local max_kept = MAX_KEPT
- if try_attr(local_conf, "plugin_attr", "log-rotate") then
+ if core.table.try_read_attr(local_conf, "plugin_attr", "log-rotate") then
local attr = local_conf.plugin_attr["log-rotate"]
Review comment:
OK. I had fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2566: chore: move try_read_attr function into table.lua (#2257)
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2566:
URL: https://github.com/apache/apisix/pull/2566#discussion_r514249000
##########
File path: apisix/core/table.lua
##########
@@ -59,6 +59,22 @@ function _M.set(tab, ...)
end
+function _M.try_read_attr(tab, ...)
Review comment:
Should also merge the `try_attr` in other plugin to this function.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2566: chore: move try_read_attr function into table.lua (#2257)
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2566:
URL: https://github.com/apache/apisix/pull/2566#discussion_r514298437
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -29,6 +28,7 @@ local core = require("apisix.core")
local hmac = require("resty.hmac")
local consumer = require("apisix.consumer")
local ngx_decode_base64 = ngx.decode_base64
+local try_read_attr = core.table.try_read_attr
Review comment:
we do not need to cache it, call `core.table.try_read_attr` is simpler.
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -29,6 +28,7 @@ local core = require("apisix.core")
local hmac = require("resty.hmac")
local consumer = require("apisix.consumer")
local ngx_decode_base64 = ngx.decode_base64
+local try_read_attr = core.table.try_read_attr
Review comment:
please fix the similar points
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] starsz commented on a change in pull request #2566: chore: move try_read_attr function into table.lua (#2257)
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #2566:
URL: https://github.com/apache/apisix/pull/2566#discussion_r514260862
##########
File path: apisix/core/table.lua
##########
@@ -59,6 +59,22 @@ function _M.set(tab, ...)
end
+function _M.try_read_attr(tab, ...)
Review comment:
Sorry, I forgot.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org