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

[5.4] no eval for RedisStore::add() [requires Redis version >= 2.6.12] #17477

Merged
merged 1 commit into from
Jan 22, 2017

Conversation

halaei
Copy link
Contributor

@halaei halaei commented Jan 22, 2017

We already need Redis >= 2.6.0 for RedisStore::add() to work. For the sake of performance, I think it is OK not to support Redis < 2.6.12. Those are old versions and migration should be easy.
The decision is yours.

@taylorotwell taylorotwell merged commit edff989 into laravel:5.4 Jan 22, 2017
@halaei
Copy link
Contributor Author

halaei commented Jan 22, 2017

Sorry sounds like there is difference between signature of \Redis::set() and \Predis\ClientInterface::set().
Please revert this @taylorotwell
@tillkruss Is there any way to call set with 'EX' and 'NX' arguments that works with both predis and phpredis?

@tillkruss
Copy link
Contributor

Yes, you can overwrite the behavior for PhpRedis in PhpRedisConnection.php.

@halaei
Copy link
Contributor Author

halaei commented Jan 22, 2017

Can we inherit a new class from \Redis class itself and match the signature with Predis?

@tillkruss
Copy link
Contributor

tillkruss commented Jan 22, 2017

No need. Just add PhpRedisConnection::set() to bend PhpRedis' set() syntax to Redis' syntax.

@tillkruss
Copy link
Contributor

Predis and PhpRedis are using different signatures for a bunch of methods. I'll work on this the next couple of days. After that you can re-submit your PR.

@halaei
Copy link
Contributor Author

halaei commented Jan 22, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants