connection with timeout option #32
Conversation
|
Change looks good to me. I think you should also add the timeout to the Can you also add a phpt to tests directory? Should be pretty straightforward, like https://github.com/nrk/phpiredis/blob/master/tests/client_001.phpt Thanks! |
|
Done :-) Have a good day. |
|
Hi, do you need anything more to merge ? Regards |
|
It looks good to me. I've asked @nrk to take a look to confirm. I'm waiting for his approval. |
| var_dump($redis); | ||
|
|
||
| --EXPECTF-- | ||
| resource(%d) of type (phpredis connection) |
There was a problem hiding this comment.
I'd change the actual test to something like:
$redis = create_phpiredis_connection('169.254.10.10', REDIS_PORT, false, 200);
usleep(500000);
--EXPECTF--
Cannot connect to host '169.254.10.10' on port [6379]This kind of test makes more sense in this case. Since the create_phpiredis_connection() utility function does a printf we should check that output which is not really clean but it works. I'm using 169.254.10.10 here which is the default static IP address in DHCP configurations when the client cannot get an IP from the DHCP server, I guess it's safe for our testing scenario.
Also, maybe we should perform the same test phpiredis_pconnect().
|
Sorry for the late response @atlantis3001 but I've been more busy than expected so I couldn't find the time to comment earlier here. All in all I'm OK with your PR, but as I've pointed out in my inline comment I'd change the very nature of the test to simulate the actual condition of a timeout and check the result. |
|
Sorry, I'm late too, I add the new tests. |
About this #31
New option when you create a phpredis_connect
phpiredis_connect($ip [, int $port, int $timeout ] );
$timeout is optionnal and is in millisecond
It's work with phpiredis_pconnect too.
example:
phpiredis_connect('127.0.0.1' , 6379, 20 );