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

[6.x] Fix phpredis "zadd" and "exists" on cluster #31838

Merged
merged 1 commit into from Mar 8, 2020

Conversation

@themsaid
Copy link
Member

themsaid commented Mar 8, 2020

This PR fixes #29637 and #30473

By not using executeRaw, the cluster connection will call the correct redis command.

@taylorotwell taylorotwell merged commit 7d9d0b5 into laravel:6.x Mar 8, 2020
7 checks passed
7 checks passed
P7.2 - Sprefer-lowest
Details
P7.2 - Sprefer-stable
Details
P7.3 - Sprefer-lowest
Details
P7.3 - Sprefer-stable
Details
P7.4 - Sprefer-lowest
Details
P7.4 - Sprefer-stable
Details
continuous-integration/styleci/pr The analysis has passed
Details
sthagen added a commit to sthagen/laravel-framework that referenced this pull request Mar 8, 2020
        fix phpredis zadd and exists on cluster (laravel#31838)
@stancl

This comment has been minimized.

Copy link
Contributor

stancl commented Mar 12, 2020

@themsaid

Removing exists() is a breaking change, because the parameters are now different. Previously passing multiple arguments was okay, but now only one argument can be passed.

My package's tests were passing and now they are failing because of that. See this: https://travis-ci.com/github/stancl/tenancy/builds/152720978#L2193

And this: https://github.com/stancl/tenancy/blob/2.x/src/StorageDrivers/RedisStorageDriver.php#L59-L61

@themsaid

This comment has been minimized.

Copy link
Member Author

themsaid commented Mar 12, 2020

@stancl but I can pass multiple keys no problem. Maybe you’re using a really old phpredis version?

@stancl

This comment has been minimized.

Copy link
Contributor

stancl commented Mar 12, 2020

@themsaid Hmm, possible. Seems like I'm using 3.1.6 in that build and the latest version is 5.2.0.

I guess I can find a way to update that, but I wonder if this would cause issues for others as well, since (for example) my code is a package.

But I see that you specified ^4.0 as phpredis version constraint, so I'll try with phpredis 4.

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

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.