Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: set minLength of redis_cluster_nodes to 1 for limit-count plugin #10612

Conversation

ikatlinsky
Copy link
Contributor

Description

As per #10596

Fixes #10596

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@@ -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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a new test case instead of change old one.
Better cover example in #10596 (comment), like

"limit-count": {
            "redis_cluster_nodes": [
              "redis-headless-service:6379"
            ],
        }

@ikatlinsky
Copy link
Contributor Author

@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

@monkeyDluffy6017 monkeyDluffy6017 merged commit c12002e into apache:master Dec 11, 2023
30 checks passed
@ikatlinsky ikatlinsky deleted the feat/limit-count-cluster-nodes-min-length branch December 18, 2023 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: as a user I want to configure limit-count plugin with single value in redis_cluster_nodes property
4 participants