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.8] Add setDriver() #28985

Merged
merged 2 commits into from
Jun 28, 2019
Merged

[5.8] Add setDriver() #28985

merged 2 commits into from
Jun 28, 2019

Conversation

stancl
Copy link
Contributor

@stancl stancl commented Jun 28, 2019

I'd like to change the Redis driver at runtime (in tests, to verify that a part of my package works both with predis and phpredis). I'm unable to do that because RedisManager seems to be a singleton whose driver property can't be changed.

>>> use Illuminate\Support\Facades\Redis;
>>> config('database.redis.client');
=> "phpredis"
>>> Redis::resolve('default');
=> Illuminate\Redis\Connections\PhpRedisConnection {#3062}
>>> config(['database.redis.client' => 'predis']);
=> null
>>> config('database.redis.client');
=> "predis"
>>> Redis::resolve('default');
=> Illuminate\Redis\Connections\PhpRedisConnection {#3058}

Changing the driver property on RedisManager should allow to switch the connection driver. All methods for resolving connections use the connector() method which returns a connector instance based on the value of the driver property:

/**
* Get the connector instance for the current driver.
*
* @return \Illuminate\Redis\Connectors\PhpRedisConnector|\Illuminate\Redis\Connectors\PredisConnector
*/
protected function connector()
{
switch ($this->driver) {
case 'predis':
return new Connectors\PredisConnector;
case 'phpredis':
return new Connectors\PhpRedisConnector;
}
}

I see that changing the Redis driver at runtime doesn't have much use in application code, unlike the database, because you can have multiple connections. However, it's impossible to test predis and phpredis support without this feature. If you don't want a setter, changing the visibility of the driver property to a public would work as well.

@taylorotwell taylorotwell merged commit 743e68f into laravel:5.8 Jun 28, 2019
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