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

[Feature] Cache queries results #32

Closed
pesseyjulien opened this issue Feb 28, 2017 · 11 comments · Fixed by #35
Closed

[Feature] Cache queries results #32

pesseyjulien opened this issue Feb 28, 2017 · 11 comments · Fixed by #35

Comments

@pesseyjulien
Copy link

Hi,

Thanks for this great bundle.

I think it would be even more awesome if we could cache the results of the doctrine queries (using redis for example), and of course, invalidate when the value changes.

What do you think ?

-Julien

@craue
Copy link
Owner

craue commented Feb 28, 2017

At first, the idea sounds good, but I'm not sure if it can be implemented in a safe way. If settings are modified by the app, cache values could be invalidated properly. But modifying them by e.g. Doctrine migrations would lead to inconsistency between cache and database.

@pesseyjulien
Copy link
Author

I agree, but what if we empty the cache in this particular case ?

@craue
Copy link
Owner

craue commented Mar 1, 2017

How? A migration was only one example.

@pesseyjulien
Copy link
Author

Sorry for the delay of my answer.

If we relay on a third-party to cache, eg Redis, then we can delete every keys in case of a migration or something similar. Or a least warn the user.

In case of an update of an existing setting, then we can just delete the corresponding redis key.

I'm mentioning the feature because the LexikTranslationBundle is doing something similar to your bundle, but with translations, and they are using a cache system.

Maybe it could be an option, deactivated by default ?

@craue
Copy link
Owner

craue commented Mar 28, 2017

How about #35?

@pesseyjulien
Copy link
Author

pesseyjulien commented Mar 28, 2017

Nice ! Thanks !

So when we do $this->get('craue_config')->set($index, $data), the cache is automatically cleared for this index or we need to do it ?

@craue
Copy link
Owner

craue commented Mar 28, 2017

This will be done by the bundle, yes. Feel free to test it out and let me know about possible improvements for the PR.

@pesseyjulien
Copy link
Author

Alright, I will ! Thanks a bunch again !

@pesseyjulien
Copy link
Author

I have just tried it using DoctrineCacheBundle and redis as cache provider, and it's working great !
Thanks a bunch for this cool new feature, it will help save a lot of queries ;)
When do you think it will be merged to the master branch ?

@craue craue closed this as completed in #35 Mar 30, 2017
@craue
Copy link
Owner

craue commented Mar 30, 2017

Aaaand merged. 😏

@pesseyjulien
Copy link
Author

Thanks again, great feature ! ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants