-
Notifications
You must be signed in to change notification settings - Fork 12
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:support redis username #76
Conversation
1d6a131
to
17ec892
Compare
17ec892
to
2674ce3
Compare
399002d
to
2327b57
Compare
2327b57
to
021dcb8
Compare
package.json
Outdated
@@ -8,7 +8,8 @@ | |||
"url": "http://github.com/auth0/limitd-redis.git" | |||
}, | |||
"scripts": { | |||
"test": "trap 'docker-compose down --remove-orphans -v' EXIT; docker-compose up -d && NODE_ENV=test nyc mocha --exit" | |||
"test-standalone": "CLUSTERED_ENV=false NODE_ENV=test nyc mocha --exit", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a huge fan of this way of running a subset of tests.
I think a better way would be to use mocha to filter for specific test file names. Mocha accepts a glob pattern for test files.
Not tested, but something like:
"test-standalone": "CLUSTERED_ENV=false NODE_ENV=test nyc mocha --exit", | |
"test-standalone": "NODE_ENV=test nyc mocha --exit '!**/*.clustermode.tests.js,**/*.standalone.tests.js'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Did it a bit different but same idea. Thanks for the suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer a different way of filtering the tests for standalone and cluster (mentioned a suggestion using mocha in a comment), but will approve to not block and leave the decision up to you 👍
By submitting a PR to this repository, you agree to the terms within the Auth0 Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.
Description
This PR adds the ability to include the username within the optional parameters of the LimitdRedis constructor to use it when connecting to a Redis Cluster.
For Standalone Redis instances, there's no need to include the username as an attribute as we support it through the connection URI.
References
Testing
Added specific tests for the username
Introduced clustered tests, which is running the same battery of tests against a clustered environment.
This change adds test coverage for new/changed/fixed functionality
Checklist