#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 |
samdark left a comment
samdark left a comment There was a problem hiding this comment.
All good but need a changelog and fixes for the small issues I've mentioned.
Cache.php Outdated
| public $redis = 'redis'; | ||
| | ||
| /** | ||
| * @var bool |
There was a problem hiding this comment.
Missing @since tag and description.
| /** | ||
| * @var array | ||
| */ | ||
| public $replicas = []; |
There was a problem hiding this comment.
Missing @since tag and description.
Cache.php Outdated
| public $replicas = []; | ||
| | ||
| /** | ||
| * @var Connection |
| * @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!. |
5 participants
For supporting Read Replica in Cache usage