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

Fixes RedisCluster constants deprecation in PHPRedis 6. #1208

Merged
merged 1 commit into from
Apr 12, 2024
Merged

Fixes RedisCluster constants deprecation in PHPRedis 6. #1208

merged 1 commit into from
Apr 12, 2024

Conversation

lucasromanojf
Copy link
Contributor

@lucasromanojf lucasromanojf commented Apr 12, 2024

When deploying a project which uses Tenancy for Laravel to Laravel Vapor using PHP 8.3 runtime (which uses PHPRedis 6) and a Redis cluster, the following error occurs:

Undefined constant RedisCluster::OPT_PREFIX

As detailed in this Laravel merged PR:

laravel/framework#48362

The PHPRedis library removed RedisCluster:: constants since 6.0.0. (...)

The author of PHPREDIS recommends not using the constants of RedisCluster anymore but using the constants in Redis instead (...)

This PR changes the use of $client:: (which can lead to the mentioned error when working with PHPRedis 6 and a Redis cluster) for constants to \Redis::.

Also, as stated in the Laravel PR mentioned above:

This is backward compatible since the underlying values of these constants are the same, so it will still work for older phpredis versions

@lucasromanojf lucasromanojf changed the title Fixes RedisCluster deprecation in PHPRedis 6. Fixes RedisCluster constants deprecation in PHPRedis 6. Apr 12, 2024
@stancl stancl merged commit d6d991c into archtechx:3.x Apr 12, 2024
3 checks passed
@stancl
Copy link
Member

stancl commented Apr 12, 2024

Thanks for the PR! Implemented this in v4 the exact same way.

@lucasromanojf
Copy link
Contributor Author

Thanks for merging it so fast!

@lucasromanojf lucasromanojf deleted the rediscluster-constants-deprecation branch April 12, 2024 18:23
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.

2 participants