8000 Redis Session Handler - Multiple Host DSN · Issue #34477 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Redis Session Handler - Multiple Host DSN #34477

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.

Sign up for GitHub

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

Jump to bottom
Closed
BeyerJC opened this issue Nov 21, 2019 · 3 comments
Closed

Redis Session Handler - Multiple Host DSN #34477

BeyerJC opened this issue Nov 21, 2019 · 3 comments
Labels
Bug Help wanted Issues and PRs which are looking for volunteers to complete them. HttpFoundation Status: Needs Review

Comments

@BeyerJC
Copy link
BeyerJC commented Nov 21, 2019 •
edited
Loading

Hello everyone!

#34177 says we can now use DSNs for configuring the session handler. For me, it works fine when using a single Redis server. But what if i try to use multiple servers ? I copied a example dsn from the cache component which looks like this :

'redis:?host[localhost]&host[localhost:6379]&host[/var/run/redis.sock:]&auth=my-password&redis_cluster=1'

or in the yaml

handler_id: 'redis:?host[localhost]&host[localhost:6379]&host[/var/run/redis.sock:]&auth=my-password&redis_cluster=1'

This causes an error because "redis:?" is not proccessed as a redis dsn.

Is my configuration wrong or is this a bug in the implementation ?

Thanks in advance !
@nicolas-grekas
Copy link
Member

Looks like something we forgot about (we check for redis:// and not only for redis: isn't it?)

PR welcome to fix the issue (the patch attached to #34177 should be a good start to know where the code needs to be adjusted.)

@nicolas-grekas nicolas-grekas added the Help wanted Issues and PRs which are looking for volunteers to complete them. label Nov 24, 2019
@BeyerJC
Copy link
Author
BeyerJC commented Nov 24, 2019

Yes, thats what i thought when looking into the class. Should be easily fixed, i will test that tomorrow and create a PR.

@BeyerJC BeyerJC mentioned this issue Dec 13, 2019
Merged
nicolas-grekas added a commit that referenced this issue Dec 13, 2019
@nicolas-grekas
bug #34967 [HttpFoundation] fix redis multi host dsn not recognized (…
212dc53 
…Jan Christoph Beyer)

This PR was merged into the 4.4 branch.

Discussion
----------

[HttpFoundation] fix redis multi host dsn not recognized

| Q             | A
| ------------- | ---
| Branch?       |  4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #34477
| License       | MIT

#34177 added support configurating session handlers with DSNs. It was no possible to pass a redis-DSN like
`redis:?host[localhost]&host[localhost:6379]&host[/var/run/redis.sock:]&auth=my-password&redis_cluster=1'`

since the check was
`case 0 === strpos($connection, 'redis://'):`

Commits
-------

81ba07a fix redis multi host dsn not recognized
@stajnert
Copy link

@WhiteRabbitDE @nicolas-grekas The same should be done in https://raw.githubusercontent.com/symfony/lock/master/Store/StoreFactory.php for lock component

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Help wanted Issues and PRs which are looking for volunteers to complete them. HttpFoundation Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@stajnert @nicolas-grekas @xabbuh @carsonbot @BeyerJC
0