You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "ikatlinsky (via GitHub)" <gi...@apache.org> on 2023/12/07 07:17:08 UTC
[PR] feat: set minLength of redis_cluster_nodes to 1 for limit-count plugin [apisix]
ikatlinsky opened a new pull request, #10612:
URL: https://github.com/apache/apisix/pull/10612
### Description
As per https://github.com/apache/apisix/issues/10596
Fixes #10596
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [x] I have added tests corresponding to this change
- [x] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX 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
Re: [PR] feat: set minLength of redis_cluster_nodes to 1 for limit-count plugin [apisix]
Posted by "moonming (via GitHub)" <gi...@apache.org>.
moonming commented on code in PR #10612:
URL: https://github.com/apache/apisix/pull/10612#discussion_r1418859071
##########
t/plugin/limit-count-redis-cluster.t:
##########
@@ -95,8 +95,7 @@ __DATA__
"policy": "redis-cluster",
"redis_timeout": 1001,
"redis_cluster_nodes": [
- "127.0.0.1:5000",
- "127.0.0.1:5001"
+ "127.0.0.1:5000"
Review Comment:
please add a new test case instead of change old one.
Better cover example in https://github.com/apache/apisix/issues/10596#issuecomment-1844685990, like
```
"limit-count": {
"redis_cluster_nodes": [
"redis-headless-service:6379"
],
}
```
--
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
Re: [PR] feat: set minLength of redis_cluster_nodes to 1 for limit-count plugin [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 merged PR #10612:
URL: https://github.com/apache/apisix/pull/10612
--
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
Re: [PR] feat: set minLength of redis_cluster_nodes to 1 for limit-count plugin [apisix]
Posted by "ikatlinsky (via GitHub)" <gi...@apache.org>.
ikatlinsky commented on PR #10612:
URL: https://github.com/apache/apisix/pull/10612#issuecomment-1846606185
@moonming Hi! I looking through the failing tests log (https://github.com/apache/apisix/actions/runs/7128370342/job/19436892955?pr=10612), can it be problem with CI action itself? Seems like there was some sort of initialization issue.
Thanks
--
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