#66 distribute the read operation on Redis Cache #132
Conversation
samdark
added
type:enhancement
|
Thank you for putting effort in the improvement of the Yii framework. In order for the framework and your solution to remain stable in the future, we have a unit test requirement in place. Therefore we can only accept your pull request if it is covered by unit tests. Could you add these please? Thanks! P.S. If you have any questions about the creation of unit tests? Don't hesitate to ask for support. More information about unit tests This is an automated comment, triggered by adding the label |
Cache.php
Outdated
| @@ -67,6 +88,21 @@ class Cache extends \yii\caching\Cache | |||
| */ | |||
| public $redis = 'redis'; | |||
|
|
|||
| /** | |||
| * @var bool | |||
There was a problem hiding this comment.
Missing @since tag and description.
Cache.php
Outdated
| /** | ||
| * @var array | ||
| */ | ||
| public $replicas = []; |
There was a problem hiding this comment.
Missing @since tag and description.
Cache.php
Outdated
| public $replicas = []; | ||
|
|
||
| /** | ||
| * @var Connection |
Cache.php
Outdated
| * @inheritdoc | ||
| * @return Connection | ||
| */ | ||
| protected function getReplica() |
There was a problem hiding this comment.
Looks like a new method. Thus, @inheritdoc doesn't make sense. Missing @since tag and description.
Cache.php
Outdated
| $config['class'] = 'yii\redis\Connection'; | ||
| } | ||
|
|
||
| //--- if hostname same, no need re-open connection |
There was a problem hiding this comment.
// If hostname is the same, there is no need re-open connection
|
I have simplified the code a bit and added more docs. |
|
seems I broke the test... :-/ |
tests/RedisCacheTest.php
Outdated
| $cache->enableReplicas = true; | ||
|
|
||
| $cache->replicas = [ | ||
| ['class' => 'yii\db\Connection'], |
There was a problem hiding this comment.
My initial intention of not using Instance::ensure() is not to break the application if replica is not properly configured, will update the test to expect exception
There was a problem hiding this comment.
exception expect to be handled by UnitTest in Instance Class on Instance::ensure() function, therefore I removed the test for invalid config.
… class, so ignore the test for replica since we applied ensure function
… class, so ignore the test for replica since we applied ensure function
|
Thank you! |
|
nice!. |
For supporting Read Replica in Cache usage