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/12/08 03:40:07 UTC
[GitHub] [apisix] qq54903099 opened a new pull request #5732: feat: consumer-restriction add reject-message
qq54903099 opened a new pull request #5732:
URL: https://github.com/apache/apisix/pull/5732
### 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. -->
### Pre-submission checklist:
<!--
Please follow the requirements:
1. Use Draft if the PR is not ready to be reviewed
2. Test is required for the feat/fix PR, unless you have a good reason
3. Doc is required for the feat PR
4. Use a new commit to resolve review instead of `push -f`
5. Use "request review" to notify the reviewer once you have resolved the review
-->
* [ ] 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] spacewander commented on a change in pull request #5732: feat: consumer-restriction add reject-message
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5732:
URL: https://github.com/apache/apisix/pull/5732#discussion_r765396247
##########
File path: t/plugin/consumer-restriction.t
##########
@@ -263,7 +263,8 @@ Authorization: Basic amFjazIwMjA6MTIzNDU2
"consumer-restriction": {
"blacklist": [
"jack1"
- ]
+ ],
+ "rejected_message": "request is forbidden"
Review comment:
@qq54903099
Please update the test to make CI pass: https://github.com/apache/apisix/runs/4465467353?check_suite_focus=true
--
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 #5732: feat: consumer-restriction add reject-message
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5732:
URL: https://github.com/apache/apisix/pull/5732#discussion_r765382147
##########
File path: t/plugin/consumer-restriction.t
##########
@@ -263,7 +263,8 @@ Authorization: Basic amFjazIwMjA6MTIzNDU2
"consumer-restriction": {
"blacklist": [
"jack1"
- ]
+ ],
+ "rejected_message": "request is forbidden"
Review comment:
This change is already a new case. But we need to make sure it can pass.
--
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] qq54903099 commented on pull request #5732: feat: consumer-restriction add reject-message
Posted by GitBox <gi...@apache.org>.
qq54903099 commented on pull request #5732:
URL: https://github.com/apache/apisix/pull/5732#issuecomment-988565449
how to chose reviewers?
--
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 #5732: feat: consumer-restriction add reject-message
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5732:
URL: https://github.com/apache/apisix/pull/5732#discussion_r764635665
##########
File path: apisix/plugins/consumer-restriction.lua
##########
@@ -105,7 +106,11 @@ local function is_method_allowed(allowed_methods, method, user)
end
local function reject(conf)
- return conf.rejected_code, { message = "The " .. conf.type .. " is forbidden." }
+ local reject_message = "The " .. conf.type .. " is forbidden." ;
Review comment:
We can check `conf.reject_message` before creating new string?
--
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] qq54903099 closed pull request #5732: feat(consumer-restriction): customize rejected_msg
Posted by GitBox <gi...@apache.org>.
qq54903099 closed pull request #5732:
URL: https://github.com/apache/apisix/pull/5732
--
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] qq54903099 commented on a change in pull request #5732: feat: consumer-restriction add reject-message
Posted by GitBox <gi...@apache.org>.
qq54903099 commented on a change in pull request #5732:
URL: https://github.com/apache/apisix/pull/5732#discussion_r764679288
##########
File path: apisix/plugins/consumer-restriction.lua
##########
@@ -105,7 +106,11 @@ local function is_method_allowed(allowed_methods, method, user)
end
local function reject(conf)
- return conf.rejected_code, { message = "The " .. conf.type .. " is forbidden." }
+ local reject_message = "The " .. conf.type .. " is forbidden." ;
Review comment:
ok
--
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] leslie-tsang commented on a change in pull request #5732: feat: consumer-restriction add reject-message
Posted by GitBox <gi...@apache.org>.
leslie-tsang commented on a change in pull request #5732:
URL: https://github.com/apache/apisix/pull/5732#discussion_r765101915
##########
File path: t/plugin/consumer-restriction.t
##########
@@ -263,7 +263,8 @@ Authorization: Basic amFjazIwMjA6MTIzNDU2
"consumer-restriction": {
"blacklist": [
"jack1"
- ]
+ ],
+ "rejected_message": "request is forbidden"
Review comment:
New test cases are needed to ensure that the new feat works as expected.
##########
File path: apisix/plugins/consumer-restriction.lua
##########
@@ -51,7 +51,8 @@ local schema = {
}
}
},
- rejected_code = {type = "integer", minimum = 200, default = 403}
+ rejected_code = {type = "integer", minimum = 200, default = 403},
+ rejected_message = {type = "string"}
Review comment:
```suggestion
rejected_msg = {type = "string"}
```
Would it be better to change `rejected_message` to `rejected_msg`? to keep it similar to other plugins.
--
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 #5732: feat(consumer-restriction): customize rejected_msg
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #5732:
URL: https://github.com/apache/apisix/pull/5732
--
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] qq54903099 commented on a change in pull request #5732: feat: consumer-restriction add reject-message
Posted by GitBox <gi...@apache.org>.
qq54903099 commented on a change in pull request #5732:
URL: https://github.com/apache/apisix/pull/5732#discussion_r764681817
##########
File path: apisix/plugins/consumer-restriction.lua
##########
@@ -105,7 +106,11 @@ local function is_method_allowed(allowed_methods, method, user)
end
local function reject(conf)
- return conf.rejected_code, { message = "The " .. conf.type .. " is forbidden." }
+ local reject_message = "The " .. conf.type .. " is forbidden." ;
Review comment:
>
ok
--
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] qq54903099 commented on a change in pull request #5732: feat: consumer-restriction add reject-message
Posted by GitBox <gi...@apache.org>.
qq54903099 commented on a change in pull request #5732:
URL: https://github.com/apache/apisix/pull/5732#discussion_r765403536
##########
File path: apisix/plugins/consumer-restriction.lua
##########
@@ -51,7 +51,8 @@ local schema = {
}
}
},
- rejected_code = {type = "integer", minimum = 200, default = 403}
+ rejected_code = {type = "integer", minimum = 200, default = 403},
+ rejected_message = {type = "string"}
Review comment:
ok
--
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