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.6] add *scan methods to work on phpredis connection v2 #24299

Closed

Conversation

ragingdave
Copy link
Contributor

This adds the adapter methods to properly utilize PhpRedis commands for scan, sscan, hscan, and zscan. fixes #24222 .... take 2.

I reorganized the methods, fixed docblocks and tried to make the changes to the tests a bit clearer. The reason for the core InteractsWithRedis changes is because setUp runs after any dataProviders so rather than having to call connect on every driver, having the configureConnections call in both the setUp and the dataProvider means that everything works. Additionally the prefix addition ensures that all commands are tested with both a prefix and non-prefixed connection as there is currently a bug awaiting this merge to be fixed as well (#24257).

@taylorotwell
Copy link
Member

Merged.

@taylorotwell
Copy link
Member

And... it broke Travis.

https://travis-ci.org/laravel/framework/builds/383206574?utm_source=email&utm_medium=notification

I'm just reverting all this again.

@taylorotwell
Copy link
Member

Reverted. Feel free to PR again if it won't break tests.

@ragingdave
Copy link
Contributor Author

Well silly Travis....I'll take another pass to try and figure out what went wrong...Nothing happened on my test run and the initial Travis Run for the commit itself...I'll check for any new commits to see if there are any conflicts.

@ragingdave
Copy link
Contributor Author

@taylorotwell Looks like the changes added in the merge changed the logic. The $counter is a pass by reference variable, which only changes after the call to the client. I will refactor to include most of the style changes you made, but the scan methods will all end up looking like the sscan version with the $client call happening and then returning the array. Additionally due to this odd nature, would you prefer a comment on those methods stating something about the pass by reference behavior?

@tillkruss
Copy link
Contributor

@ragingdave: Yeah, add a comment, it's easy to remove or refactor.

@saurovh
Copy link

saurovh commented Jun 18, 2018

How should this change be used in laravel 5.4 ?

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.

4 participants