Skip to content

Conversation

@TheLevti
Copy link
Contributor

Added documentation about how to use phpredis's compression and serialization options. Also improved the documentation of other phpredis connection parameters.

Added documentation about how to use phpredis's compression and serialization options. Also improved the documentation of other phpredis connection parameters.
@taylorotwell
Copy link
Member

@TheLevti why does the serialization and compression stuff have to be nested within an options key? I am not seeing that in the code or am I missing something?

@TheLevti
Copy link
Contributor Author

TheLevti commented Jan 17, 2022

@TheLevti why does the serialization and compression stuff have to be nested within an options key? I am not seeing that in the code or am I missing something?

    /**
     * Create a new clustered PhpRedis connection.
     *
     * @param  array  $config
     * @param  array  $options
     * @return \Illuminate\Redis\Connections\PhpRedisConnection
     */
    public function connect(array $config, array $options)
    {
        $connector = function () use ($config, $options) {
            return $this->createClient(array_merge(
                $config, $options, Arr::pull($config, 'options', [])
            ));
        };

        return new PhpRedisConnection($connector(), $connector, $config);
    }

For cluster its expected in the options sub array, while for single instance a possible options sub array is merged with the outer config array. So in the end you can put those in both places, makes no difference on single instance connection configs.

The whole config set up for phpredis in single and cluster mode is very confusing and messy. I would like to improve that in a future MR. Currently specific params can be placed in so many different places and its hard to understand where to put them. For example on the cluster config the failover behaviour is set globally although this should be per cluster connection configurable.

To be consistent with the cluster connection expectations, the single instance example also has them in the options sub array.

https://github.com/TheLevti/framework/blob/808e0d3b5cb72409c9d2c419ab1271b69b674020/tests/Redis/RedisConnectionTest.php#L850 all the tests have them there as well.

@taylorotwell
Copy link
Member

I've documented this now. Thanks.

@TheLevti TheLevti deleted the patch-1 branch January 18, 2022 19:17
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