Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support multipart/form #32

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

Conversation

zhucebuliaopx
Copy link

add new feature==>support multipart/form ,so we can upload file by requests like python requests

@@ -0,0 +1,330 @@
--https://github.com/pytpeng/lua-multipart
Copy link
Owner

Choose a reason for hiding this comment

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

Why not just manage this dependency by LuaRocks/OPM rather than copy it.

Copy link
Author

Choose a reason for hiding this comment

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

upstream code not merge my pull request yet,so left the problem,Kong/lua-multipart#27

Copy link
Owner

Choose a reason for hiding this comment

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

I think we should suspend this MR until your MR merged by Kong.

Copy link
Author

Choose a reason for hiding this comment

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

thx,this is better

@@ -69,11 +69,21 @@ local function prepare(url_parts, session, config)
local content
local json = config.json
local body = config.body
local files = config.files

if json then
content = cjson.encode(json)
headers["content-length"] = #content
headers["content-type"] = "application/json"
Copy link
Owner

Choose a reason for hiding this comment

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

Better to leave an empty line here.

Copy link
Author

Choose a reason for hiding this comment

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

ok

lib/resty/requests/request.lua Outdated Show resolved Hide resolved
if not content_type then
content_type = "multipart/form-data; boundary="..util.choose_boundary()
end
local multipart_body= util.make_multipart_body(files, content_type)
Copy link
Owner

Choose a reason for hiding this comment

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

Style: a space character should be kept before =.

local multipart_body= util.make_multipart_body(files, content_type)
headers["content-type"] = content_type
headers["content-length"] = #multipart_body
content = multipart_body
Copy link
Owner

Choose a reason for hiding this comment

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

Ditto, leave an empty line here.

local ngx_gsub = ngx.re.gsub
local base64 = ngx.encode_base64
local Multipart = require("resty.requests.multipart")
Copy link
Owner

Choose a reason for hiding this comment

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

Well, please keep all require statements at the top of file, also, do not use capital letters.

@@ -131,6 +136,9 @@ local function set_config(opts)
-- 4) body
config.body = opts.body

-- 4.1) files
config.files = opts.files
Copy link
Owner

Choose a reason for hiding this comment

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

Since we add an extra to config, adjusting the table slot size is necessary.

lib/resty/requests/util.lua Outdated Show resolved Hide resolved
local function choose_boundary()
return str_sub(tostring({}), 10)
end

Copy link
Owner

Choose a reason for hiding this comment

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

Two empty lines are needed to separate functions.

for _, v in ipairs(files) do
m:set_simple(v[1], v[2], v[3], v[4])
local m = multipart("", content_type)
for i=1,#files do
Copy link
Owner

Choose a reason for hiding this comment

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

Style: keep a space before #files.

Copy link
Author

Choose a reason for hiding this comment

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

@pytpeng
Could you add some test cases to cover this feature?

yeah

@tokers
Copy link
Owner

tokers commented Mar 23, 2020

@pytpeng
Could you add some test cases to cover this feature?

@zhucebuliaopx
Copy link
Author

zhucebuliaopx commented Mar 31, 2020 via email

@tokers
Copy link
Owner

tokers commented Mar 31, 2020

@pytpeng
Sorry for the long delay. I'm somewhat busy this time. Will review it ASAP.

@tokers
Copy link
Owner

tokers commented Apr 8, 2020

@pytpeng
Some of style problems should be fixed else.

@zhucebuliaopx
Copy link
Author

@tokers
remove dependencies by Kong/lua-multipart, add new test cases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants