Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distributed caches are mem caches #30072

Closed

Conversation

ChristophWurst
Copy link
Member

Memcached and Redis are instances of IMemcache. That interface is
superior as it has the fancy inc and dec helpers.

For app development it's nice if we can use memcache without an instanceof check.

Memcached and Redis are intances of IMemcache. That interface is
superior as it has the fancy inc and dec helpers.

Signed-off-by: Christoph Wurst <[email protected]>
@ChristophWurst ChristophWurst added enhancement 3. to review Waiting for reviews integration feature: caching Related to our caching system: scssCacher, jsCombiner... labels Dec 3, 2021
@ChristophWurst ChristophWurst added this to the Nextcloud 24 milestone Dec 3, 2021
@ChristophWurst ChristophWurst requested a review from a team December 3, 2021 11:53
@ChristophWurst ChristophWurst self-assigned this Dec 3, 2021
@ChristophWurst ChristophWurst requested review from nickvergessen, ArtificialOwl and come-nc and removed request for a team December 3, 2021 11:53
@come-nc
Copy link
Contributor

come-nc commented Dec 6, 2021

Lots of tests needs to be adapted it seems. I do not understand the psalm error: "Type OCP\IMemcache for $lockingCache is always OCP\IMemcache".
Where do me see the file:line information for psalm errors from the CI?

Comment on lines +321 to +322
$cache = $this->cache = $this->createMock(IMemcache::class);
;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$cache = $this->cache = $this->createMock(IMemcache::class);
;
$cache = $this->cache = $this->createMock(IMemcache::class);

Comment on lines +88 to +89
$this->cache = $this->cache = $this->createMock(IMemcache::class);
;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$this->cache = $this->cache = $this->createMock(IMemcache::class);
;
$this->cache = $this->createMock(IMemcache::class);

@@ -561,7 +561,7 @@ public function testSingleStorageDeleteFolderFail() {
*/
public function testShouldMoveToTrash($mountPoint, $path, $userExists, $appDisablesTrash, $expected) {
$fileID = 1;
$cache = $this->createMock(ICache::class);
$cache = $this->cache = $this->createMock(IMemcache::class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$cache = $this->cache = $this->createMock(IMemcache::class);
$cache = $this->createMock(IMemcache::class);

Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a case of batch replace gone wrong.
Lots of superflous "$this->cache =" and duplicated ";".

@nickvergessen nickvergessen added 1. to develop Accepted and waiting to be taken care of and removed 3. to review Waiting for reviews labels Dec 7, 2021
@skjnldsv skjnldsv mentioned this pull request Mar 24, 2022
@blizzz blizzz mentioned this pull request Mar 31, 2022
@ChristophWurst ChristophWurst deleted the enhancement/cache-factory-distributed-memcache branch April 1, 2022 15:07
@ChristophWurst
Copy link
Member Author

I've raised #31787 so this can be picked up later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of enhancement feature: caching Related to our caching system: scssCacher, jsCombiner... integration
Projects
Development

Successfully merging this pull request may close these issues.

3 participants