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 2021/10/28 07:23:32 UTC
[GitHub] [apisix] Xunzhuo opened a new pull request #5354: feat(limit-conn): support multiple variables as key
Xunzhuo opened a new pull request #5354:
URL: https://github.com/apache/apisix/pull/5354
### 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. -->
support multiple variables as key
### Pre-submission checklist:
* [ ] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [ ] 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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Xunzhuo removed a comment on pull request #5354: feat(limit-conn): support multiple variables as key
Posted by GitBox <gi...@apache.org>.
Xunzhuo removed a comment on pull request #5354:
URL: https://github.com/apache/apisix/pull/5354#issuecomment-957060522
@spacewander Coud you take a look at the CI failure?
--
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] Xunzhuo commented on a change in pull request #5354: feat(limit-conn): support multiple variables as key
Posted by GitBox <gi...@apache.org>.
Xunzhuo commented on a change in pull request #5354:
URL: https://github.com/apache/apisix/pull/5354#discussion_r740774269
##########
File path: t/plugin/limit-conn2.t
##########
@@ -308,3 +308,134 @@ request latency is nil
--- error_code: 400
--- response_body
{"error_msg":"failed to check the configuration of plugin limit-conn err: property \"rejected_msg\" validation failed: string too short, expected at least 1, got 0"}
+
+
+
+=== TEST 9: set key type to var_combination
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-conn": {
+ "conn": 2,
+ "burst": 1,
+ "default_conn_delay": 0.1,
+ "rejected_code": 503,
+ "key": "$http_a $http_b",
Review comment:
@tzssangglass would you like to take a look at this PR again? doc updated.
--
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 merged pull request #5354: feat(limit-conn): support multiple variables as key
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #5354:
URL: https://github.com/apache/apisix/pull/5354
--
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] Xunzhuo commented on pull request #5354: feat(limit-conn): support multiple variables as key
Posted by GitBox <gi...@apache.org>.
Xunzhuo commented on pull request #5354:
URL: https://github.com/apache/apisix/pull/5354#issuecomment-957060522
@spacewander Coud you take a look at the CI failure?
--
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] Xunzhuo commented on a change in pull request #5354: feat(limit-conn): support multiple variables as key
Posted by GitBox <gi...@apache.org>.
Xunzhuo commented on a change in pull request #5354:
URL: https://github.com/apache/apisix/pull/5354#discussion_r740774269
##########
File path: t/plugin/limit-conn2.t
##########
@@ -308,3 +308,134 @@ request latency is nil
--- error_code: 400
--- response_body
{"error_msg":"failed to check the configuration of plugin limit-conn err: property \"rejected_msg\" validation failed: string too short, expected at least 1, got 0"}
+
+
+
+=== TEST 9: set key type to var_combination
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-conn": {
+ "conn": 2,
+ "burst": 1,
+ "default_conn_delay": 0.1,
+ "rejected_code": 503,
+ "key": "$http_a $http_b",
Review comment:
@tzssangglass would you like to take a look at this PR again? doc updated.
--
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] Xunzhuo commented on a change in pull request #5354: feat(limit-conn): support multiple variables as key
Posted by GitBox <gi...@apache.org>.
Xunzhuo commented on a change in pull request #5354:
URL: https://github.com/apache/apisix/pull/5354#discussion_r740288287
##########
File path: t/plugin/limit-conn2.t
##########
@@ -308,3 +308,134 @@ request latency is nil
--- error_code: 400
--- response_body
{"error_msg":"failed to check the configuration of plugin limit-conn err: property \"rejected_msg\" validation failed: string too short, expected at least 1, got 0"}
+
+
+
+=== TEST 9: set key type to var_combination
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-conn": {
+ "conn": 2,
+ "burst": 1,
+ "default_conn_delay": 0.1,
+ "rejected_code": 503,
+ "key": "$http_a $http_b",
Review comment:
Thanks for your tips, two variables are separated by a space, doc updated.
--
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] Xunzhuo removed a comment on pull request #5354: feat(limit-conn): support multiple variables as key
Posted by GitBox <gi...@apache.org>.
Xunzhuo removed a comment on pull request #5354:
URL: https://github.com/apache/apisix/pull/5354#issuecomment-957060522
@spacewander Coud you take a look at the CI failure?
--
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] tzssangglass commented on a change in pull request #5354: feat(limit-conn): support multiple variables as key
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #5354:
URL: https://github.com/apache/apisix/pull/5354#discussion_r740135806
##########
File path: t/plugin/limit-conn2.t
##########
@@ -308,3 +308,134 @@ request latency is nil
--- error_code: 400
--- response_body
{"error_msg":"failed to check the configuration of plugin limit-conn err: property \"rejected_msg\" validation failed: string too short, expected at least 1, got 0"}
+
+
+
+=== TEST 9: set key type to var_combination
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-conn": {
+ "conn": 2,
+ "burst": 1,
+ "default_conn_delay": 0.1,
+ "rejected_code": 503,
+ "key": "$http_a $http_b",
Review comment:
Are the two variables separated by a space? Depending on the document it is separated by `|`.
--
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] Xunzhuo commented on pull request #5354: feat(limit-conn): support multiple variables as key
Posted by GitBox <gi...@apache.org>.
Xunzhuo commented on pull request #5354:
URL: https://github.com/apache/apisix/pull/5354#issuecomment-957060522
@spacewander Coud you take a look at the CI failure?
--
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] Xunzhuo commented on pull request #5354: feat(limit-conn): support multiple variables as key
Posted by GitBox <gi...@apache.org>.
Xunzhuo commented on pull request #5354:
URL: https://github.com/apache/apisix/pull/5354#issuecomment-957060522
@spacewander Coud you take a look at the CI failure?
--
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 merged pull request #5354: feat(limit-conn): support multiple variables as key
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #5354:
URL: https://github.com/apache/apisix/pull/5354
--
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] Xunzhuo commented on a change in pull request #5354: feat(limit-conn): support multiple variables as key
Posted by GitBox <gi...@apache.org>.
Xunzhuo commented on a change in pull request #5354:
URL: https://github.com/apache/apisix/pull/5354#discussion_r740774269
##########
File path: t/plugin/limit-conn2.t
##########
@@ -308,3 +308,134 @@ request latency is nil
--- error_code: 400
--- response_body
{"error_msg":"failed to check the configuration of plugin limit-conn err: property \"rejected_msg\" validation failed: string too short, expected at least 1, got 0"}
+
+
+
+=== TEST 9: set key type to var_combination
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-conn": {
+ "conn": 2,
+ "burst": 1,
+ "default_conn_delay": 0.1,
+ "rejected_code": 503,
+ "key": "$http_a $http_b",
Review comment:
@tzssangglass would you like to take a look at this PR again? doc updated.
--
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 merged pull request #5354: feat(limit-conn): support multiple variables as key
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #5354:
URL: https://github.com/apache/apisix/pull/5354
--
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] Xunzhuo removed a comment on pull request #5354: feat(limit-conn): support multiple variables as key
Posted by GitBox <gi...@apache.org>.
Xunzhuo removed a comment on pull request #5354:
URL: https://github.com/apache/apisix/pull/5354#issuecomment-957060522
@spacewander Coud you take a look at the CI failure?
--
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