-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix: revert parsing username via URI due to potential breaking changes #1134
Conversation
## [4.17.1](v4.17.0...v4.17.1) (2020-05-16) ### Bug Fixes * revert parsing username via URI due to potential breaking changes ([#1134](#1134)) ([225ef45](225ef45))
🎉 This PR is included in version 4.17.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Could this be brought back with a flag to allow proper URI parsing? Or in a v5 release? |
@billinghamj I thought we can launch this in v5 but a flag sounds good to me. |
@luin what do you think about just skipping the username assignment if the username is default?
I think that would preserve backward compatibility for older servers without requiring a flag or a major release. |
@bpo Actually people may put arbitrary usernames ex "user" for whatever reasons in the URL as they thought it doesn't matter and will be ignored by ioredis. So your approach works in most normal cases but still introduces risks of breaking their assumption. It's debatable what kind of changes should be considered as breaking changes, but I think we should be cautious anyway so I'd prefer adding a flag or next major release. |
@luin do you still plan to put it back? |
## [4.17.1](redis/ioredis@v4.17.0...v4.17.1) (2020-05-16) ### Bug Fixes * revert parsing username via URI due to potential breaking changes ([#1134](redis/ioredis#1134)) ([225ef45](redis/ioredis@225ef45))
Closes #1132.
Previously, users can specify the username in Redis connect URI unintentionally so parsing username leads potential breaking changes in this case. We still support
username
parameter so users can opt-in ACL explicitly.