[Cache] Add support for ACL auth in RedisAdapter #45313
Conversation
There was a problem hiding this comment.
I have a concern: right now, the user part of the credentials is simply ignored.
On eg heroku, the provided DSN always uses h as username. This PR would break such setups, isn't it? Or does Redis ignore the username when the password is correct (no-ACL)?
What about going with a user param in the query string instead?
|
I can answer part of my questions with https://redis.io/commands/auth:
Could you also please see how your patch behaves on older versions of Redis? |
08f2b63 to
5d04d23
Compare
|
This feature not breaks BC with redis v5 and redis v4. |
52ba5d4 to
9de08db
Compare
|
I just pushed some changes (see 2nd commit). Tests fails, see CI. I'm able to reproduce the failures locally by running Failures were already there before my changes ;) |
|
@nicolas-grekas I neet to fix tests for AppVeyor CI? |
|
On Linux, see "Integration / Tests" line. |
|
An easy fix might be to remove testDefaultUserPasswordAuth |
9de08db to
26a4c60
Compare
This PR was merged into the 4.4 branch. Discussion ---------- [Cache] fix error handling when using Redis | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #45325 | License | MIT | Doc PR | - Spotted while working on #45313 I won't be able to add tests here, the situations are too edgy. Commits ------- 3c59e0f [Cache] fix error handling
26a4c60 to
722830d
Compare
|
Thank you @gam6itko. |
Added support of DSN
username:passwordsection in RedisAdapter which allows using ACL AUTH