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/09/11 20:38:08 UTC
[GitHub] [apisix] swayamraina opened a new pull request #2221: send file contents and not file name
swayamraina opened a new pull request #2221:
URL: https://github.com/apache/apisix/pull/2221
### What this PR does / why we need it:
implements https://github.com/apache/apisix/issues/2196
----------------------------------------------------------------
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] github-actions[bot] closed pull request #2221: send file contents and not file name
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #2221:
URL: https://github.com/apache/apisix/pull/2221
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2221: send file contents and not file name
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2221:
URL: https://github.com/apache/apisix/pull/2221#discussion_r551660428
##########
File path: apisix/utils/log-util.lua
##########
@@ -64,7 +66,10 @@ local function get_full_log(ngx, conf)
else
local body_file = ngx.req.get_body_file()
if body_file then
- log.request.body_file = body_file
+ local file = open(body_file, "rb")
+ local content = file:read "*a"
+ file:close()
+ log.request.body_file = content
Review comment:
We should use a new field to store the content to avoid break change.
##########
File path: apisix/utils/log-util.lua
##########
@@ -64,7 +66,10 @@ local function get_full_log(ngx, conf)
else
local body_file = ngx.req.get_body_file()
if body_file then
- log.request.body_file = body_file
+ local file = open(body_file, "rb")
Review comment:
Need to check if `open` is successful to handle something like disk problem.
##########
File path: apisix/utils/log-util.lua
##########
@@ -64,7 +66,10 @@ local function get_full_log(ngx, conf)
else
local body_file = ngx.req.get_body_file()
if body_file then
- log.request.body_file = body_file
+ local file = open(body_file, "rb")
+ local content = file:read "*a"
Review comment:
Need to check if `read` is successful like `local content, err = file:read "*a"`
----------------------------------------------------------------
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 #2221: send file contents and not file name
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2221:
URL: https://github.com/apache/apisix/pull/2221#discussion_r488333965
##########
File path: apisix/utils/log-util.lua
##########
@@ -64,7 +64,11 @@ local function get_full_log(ngx, conf)
else
local body_file = ngx.req.get_body_file()
if body_file then
- log.request.body_file = body_file
+ local lines = {}
Review comment:
https://stackoverflow.com/questions/11201262/how-to-read-data-from-a-file-in-lua
Read the file content in this way, is the right way:
```lua
local open = io.open
local function read_file(path)
local file = open(path, "rb") -- r read mode and b binary mode
if not file then return nil end
local content = file:read "*a" -- *a or *all reads the whole file
file:close()
return content
end
local fileContent = read_file("foo.html");
print (fileContent);
```
----------------------------------------------------------------
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] github-actions[bot] commented on pull request #2221: send file contents and not file name
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #2221:
URL: https://github.com/apache/apisix/pull/2221#issuecomment-924777722
This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] swayamraina commented on a change in pull request #2221: send file contents and not file name
Posted by GitBox <gi...@apache.org>.
swayamraina commented on a change in pull request #2221:
URL: https://github.com/apache/apisix/pull/2221#discussion_r490512498
##########
File path: apisix/utils/log-util.lua
##########
@@ -64,7 +64,11 @@ local function get_full_log(ngx, conf)
else
local body_file = ngx.req.get_body_file()
if body_file then
- log.request.body_file = body_file
+ local lines = {}
Review comment:
`if not file then return nil end`
do we need this part of the code? if the body is not present then can we not assume the data will be present in the file?
----------------------------------------------------------------
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] github-actions[bot] commented on pull request #2221: send file contents and not file name
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #2221:
URL: https://github.com/apache/apisix/pull/2221#issuecomment-904504148
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2221: send file contents and not file name
Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on pull request #2221:
URL: https://github.com/apache/apisix/pull/2221#issuecomment-783064896
Hi @swayamraina could you have a look at the comments
----------------------------------------------------------------
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] moonming commented on pull request #2221: send file contents and not file name
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #2221:
URL: https://github.com/apache/apisix/pull/2221#issuecomment-754300511
ping @membphis
----------------------------------------------------------------
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] tokers commented on pull request #2221: send file contents and not file name
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2221:
URL: https://github.com/apache/apisix/pull/2221#issuecomment-783291958
Better to add some cases to cover it.
----------------------------------------------------------------
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 #2221: send file contents and not file name
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2221:
URL: https://github.com/apache/apisix/pull/2221#discussion_r488333965
##########
File path: apisix/utils/log-util.lua
##########
@@ -64,7 +64,11 @@ local function get_full_log(ngx, conf)
else
local body_file = ngx.req.get_body_file()
if body_file then
- log.request.body_file = body_file
+ local lines = {}
Review comment:
https://stackoverflow.com/questions/11201262/how-to-read-data-from-a-file-in-lua
Read the file content in this way, here is the right way:
```lua
local open = io.open
local function read_file(path)
local file = open(path, "rb") -- r read mode and b binary mode
if not file then return nil end
local content = file:read "*a" -- *a or *all reads the whole file
file:close()
return content
end
local fileContent = read_file("foo.html");
print (fileContent);
```
----------------------------------------------------------------
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