feat: use luasocket instead of curl in etcd.lua #2965
Conversation
.travis/apisix_cli_test.sh
Outdated
| prefix: "/apisix" | ||
| ' > conf/config.yaml | ||
|
|
||
| make init &>/tmp/apisix_temp & |
There was a problem hiding this comment.
Why need to run in the background?
There was a problem hiding this comment.
Because the exit code of make init is 1. It will terminate the shell.
There was a problem hiding this comment.
@starsz
You can try this way:
apisix/.travis/apisix_cli_test_in_ci.sh
Line 43 in c41aa85
.travis/apisix_cli_test.sh
Outdated
|
|
||
| export ETCDCTL_API=3 | ||
| etcdctl version | ||
| etcdctl --endpoints=127.0.0.1:2379 user add "root:apache-api6" |
There was a problem hiding this comment.
Need to clean the user/role after running the test
| @@ -106,9 +111,6 @@ function _M.init(env, show_output) | |||
|
|
|||
| local etcd_conf = yaml_conf.etcd | |||
|
|
|||
| local timeout = etcd_conf.timeout or 3 | |||
There was a problem hiding this comment.
Should allow configuring timeout in the new way
|
|
||
| local body, _, err = dkjson.decode(res) | ||
| if err then | ||
| local res, err = http.request(version_url) |
There was a problem hiding this comment.
Look like this library doesn't support max-timeout? Only operation level timeout is supported.
There was a problem hiding this comment.
Right. The library doesn't support max-timeout.
apisix/cli/etcd.lua
Outdated
| if err then | ||
| local res, err = http.request(version_url) | ||
| if err and type(err) == "string" then | ||
| errmsg = str_format("request etcd endpoint \'%s\' error, %s\n", host, err) |
apisix/cli/etcd.lua
Outdated
|
|
||
| -- err is string type | ||
| if err and type(err) == "string" then | ||
| errmsg = str_format("request etcd endpoint \"%s\" error, %s\n", host, err) |
apisix/cli/etcd.lua
Outdated
| sink = ltn12.sink.table(response_body), | ||
| headers = headers} | ||
| if err and type(err) == "string" then | ||
| errmsg = str_format("request etcd endpoint \"%s\" error, %s\n", host, err) |
|
Hi, @spacewander, please have a look when you are free. |
apisix/cli/etcd.lua
Outdated
| sink = ltn12.sink.table(response_body), | ||
| headers = {["Content-Length"] = #post_json_auth}} | ||
| -- err is string type | ||
| if err and type(err) == "string" then |
There was a problem hiding this comment.
Better to add more info about the err handle, so that people can know what you are doing without looking into luasocket's doc.
There was a problem hiding this comment.
@spacewander Sorry, I can't get your point.
That we need to distinguish the connection refused and timeout
There was a problem hiding this comment.
I mean, why we need to check the type of err? We need to clarify it in the comment, so that people can know it without searching luasocket's doc.
There was a problem hiding this comment.
I think it would be better to use if not res then handle err? Since the first argument is nil when error happened. This way is more normal.
There was a problem hiding this comment.
Well. Agree with you.
| @@ -179,14 +179,14 @@ function _M.init(env, show_output) | |||
|
|
|||
| local post_json_auth = dkjson.encode(json_auth) | |||
| local response_body = {} | |||
| local _, err = http.request{url = auth_url, method = "POST", | |||
| local res, err = http.request{url = auth_url, method = "POST", | |||
| source = ltn12.source.string(post_json_auth), | |||
| sink = ltn12.sink.table(response_body), | |||
| headers = {["Content-Length"] = #post_json_auth}} | |||
| -- In case of failure, request returns nil followed by an error message. | |||
| -- Else the first return value is just the number 1 | |||
There was a problem hiding this comment.
The first return value isn't the response body?
There was a problem hiding this comment.
Yep. It has two forms.
You can see more details about it.
In case of failure, the function returns nil followed by an error message. If successful, the simple form returns the response body as a string, followed by the response status code, the response headers and the response status line. The generic function returns the same information, except the first return value is just the number 1 (the body goes to the sink).
| headers["Authorization"] = auth_token | ||
| end | ||
|
|
||
| local res, err = http.request{url = put_url, method = "POST", |
There was a problem hiding this comment.
style: I prefer http.request({...}), it is easy to read
What this PR does / why we need it:
fix: #2818
fix: #2718
Pre-submission checklist: