Leverage "options.parameters" config in PhpredisClientFactory::create()

[phansys]

Q	A
Branch	master
Bug fix?	no
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	n/a
License	MIT
Doc PR	n/a

[phansys]

Without these changes, parameters provided at snc_redis.clients.{name}.options.parameters are completely ignored within "phpredis". If this was the desired behavior, I think the current branch is right; otherwise, this must be considered a fix and the base should be updated to target 2.1.
Thank you in advance.

[B-Galati]

Thanks @phansys! Could you target 2.1 please?

[phansys]

Updated target to 2.1. I'm working on the tests in order to fix them.

[phansys]

I had to use the full service definition in tests ("snc_redis.phpredis.default") instead of the short alias ("snc_redis.default" ) in tests since it seems to be unavailable in tests for phpredis. Do you have any clue about this?

SncRedisBundle/DependencyInjection/SncRedisExtension.php

Line 300 in 3f64274

$container->setAlias(sprintf('snc_redis.%s', $client['alias']), new Alias($phpredisId, true));

[phansys]

Given my last comment, I think this PR should wait for #506 in order to fix the service definition.

[B-Galati]

It's ok if you just use snc_redis.phpredis.default service id for tests, this will be fixed in future 3.0.0 where only short id will be kept.

[phansys]

This PR should wait #507 to fix the usage of phpredis through Client class.

[B-Galati]

I don't get why we did not have this error before 🤔

[B-Galati]

I get it, the test was not executed at all before 😅
Thank you for moving it 👍

[phansys]

@B-Galati, in order to get this PR merged and separated concerns, I've restored PhpredisClientFactoryTest to its original path (keeping the changes related to this PR). I've created #508 in order to address the location change.
Thank you in advance.

[B-Galati]

@phansys Please keep everything in this one as it is related in some way IMHO.

Also tests that depends on a specific phpredis version are usually handled like so:

SncRedisBundle/Tests/Client/Phpredis/ClientTest.php

Lines 18 to 22 in 521d237

	if (!extension_loaded('redis')) {
	$this->markTestSkipped('This test needs the PHP Redis extension to work');
	} elseif (version_compare(phpversion('redis'), '4.0.0') >= 0) {
	$this->markTestSkipped('This test cannot be executed on Redis extension version ' . phpversion('redis'));
	}

No need to bother with redis4 group or something like this.

[phansys]

Done @B-Galati.

[B-Galati]

@phansys Awesome! Thanks a lot!