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

Allow a model to optionally specify a maximum cache timeout #274

Closed
wants to merge 2 commits into from
Closed

Allow a model to optionally specify a maximum cache timeout #274

wants to merge 2 commits into from

Conversation

uintaam
Copy link

@uintaam uintaam commented Jul 23, 2019

We are in the process of migrating some legacy parts of our code base in to Laravel and to use cached models, some of this legacy code uses raw queries to update data we want to use cached models with. This gives us a way to move to cached model code in Laravel while we migrate the legacy raw queries; by adding a single protected static $maxCacheTimeout = 300; to our cacheable model we can be sure that the raw updates will eventually, 300s from original cache, be reflected on the cached frontend. Allowing this to be defined per model means we can use fully cached models on new code/tables and use caching as we migrate older code over.

@coveralls
Copy link

coveralls commented Jul 23, 2019

Coverage Status

Coverage decreased (-0.1%) to 95.361% when pulling c534465 on zaikoio:feature/cache_timeouts into 949697b on GeneaLabs:master.

@mikebronner
Copy link
Owner

mikebronner commented Jul 23, 2019

@drewjw81 Thanks for submitting this. However, I don't believe cache timeout is the right way to go. It is much better that you manually invalidate the respective models after you run the raw SQL queries. Just be sure to invalidate all models that are affected by this query, and you should be good to go.

From shell script:

php artisan modelCache:clear --model=App\Model

From within Laravel:

app("artisan")->call("modelCache:clear --model=App\Model");

That will be much more reliable, easier to maintain, and more performant. Because of those reasons I am currently not considering to add cache timeouts. If the above does NOT work for you, please let me know why not, and we'll continue the discussion. :)

I know it's a lot of work to submit the PR, thanks for taking the initiative!

@mikebronner mikebronner self-assigned this Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants