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/12/21 06:30:12 UTC
[GitHub] [apisix] tokers opened a new pull request #3088: feat: add control api for plugin server-info
tokers opened a new pull request #3088:
URL: https://github.com/apache/apisix/pull/3088
### 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. -->
Previously we added plugin server-info without exposing API since the same port problem of admin api and proxy. Now we have Control API support so we expose server-info API through Control API. People now can insight the server info about APISIX instance easily.
### 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?
* [x] Have you modified the corresponding document?
* [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
----------------------------------------------------------------
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 #3088: feat: add control api for plugin server-info
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3088:
URL: https://github.com/apache/apisix/pull/3088#discussion_r546534487
##########
File path: apisix/plugins/server-info.lua
##########
@@ -99,6 +99,17 @@ local function get()
end
+local function get_server_info()
+ local info, err = get()
Review comment:
Look like we need to way to store the `boot_time` in shdict when `etcd` is not used. Otherwise the `boot_time` will change when plugin reloads.
----------------------------------------------------------------
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 merged pull request #3088: feat: add control api for plugin server-info
Posted by GitBox <gi...@apache.org>.
membphis merged pull request #3088:
URL: https://github.com/apache/apisix/pull/3088
----------------------------------------------------------------
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 #3088: feat: add control api for plugin server-info
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #3088:
URL: https://github.com/apache/apisix/pull/3088#discussion_r546553530
##########
File path: doc/control-api.md
##########
@@ -49,3 +49,9 @@ Here is the supported API:
Introduced since `v2.2`.
Return the jsonschema used by this APISIX instance.
+
+### GET /v1/server_info
Review comment:
I think it would be better to put this to plugin's own section. So that the API in this page is builtin and will be provided whether you use the plugin or not.
##########
File path: t/plugin/server-info.t
##########
@@ -133,3 +133,54 @@ integral
[error]
--- error_log
timer created to report server info, interval: 60
+
+
+
+=== TEST 3: get server_info from plugin control API
+--- yaml_config
+apisix:
+ id: 123456
+ enable_control: true
Review comment:
This configuration may confuses beginner, because the control server in the test is hardcoded in APISIX.pm, instead of generating from config.yaml.
----------------------------------------------------------------
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 a change in pull request #3088: feat: add control api for plugin server-info
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #3088:
URL: https://github.com/apache/apisix/pull/3088#discussion_r547012511
##########
File path: doc/control-api.md
##########
@@ -49,3 +49,9 @@ Here is the supported API:
Introduced since `v2.2`.
Return the jsonschema used by this APISIX instance.
+
+### GET /v1/server_info
Review comment:
Fixed.
##########
File path: apisix/plugins/server-info.lua
##########
@@ -99,6 +99,17 @@ local function get()
end
+local function get_server_info()
+ local info, err = get()
Review comment:
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] membphis commented on a change in pull request #3088: feat: add control api for plugin server-info
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #3088:
URL: https://github.com/apache/apisix/pull/3088#discussion_r547041136
##########
File path: t/plugin/server-info.t
##########
@@ -133,3 +133,51 @@ integral
[error]
--- error_log
timer created to report server info, interval: 60
+
+
+
+=== TEST 3: get server_info from plugin control API
+--- yaml_config
+apisix:
+ id: 123456
+plugins:
+ - server-info
+--- config
+location /t {
+ content_by_lua_block {
+ local json_decode = require("apisix.core").json.decode
Review comment:
https://github.com/api7/test-toolkit
we can use this `json` to print the sorted data. we can do this in a new PR
----------------------------------------------------------------
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 a change in pull request #3088: feat: add control api for plugin server-info
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #3088:
URL: https://github.com/apache/apisix/pull/3088#discussion_r547012436
##########
File path: t/plugin/server-info.t
##########
@@ -133,3 +133,54 @@ integral
[error]
--- error_log
timer created to report server info, interval: 60
+
+
+
+=== TEST 3: get server_info from plugin control API
+--- yaml_config
+apisix:
+ id: 123456
+ enable_control: true
Review comment:
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] tokers commented on a change in pull request #3088: feat: add control api for plugin server-info
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #3088:
URL: https://github.com/apache/apisix/pull/3088#discussion_r546536232
##########
File path: apisix/plugins/server-info.lua
##########
@@ -99,6 +99,17 @@ local function get()
end
+local function get_server_info()
+ local info, err = get()
Review comment:
That's really, i will change 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