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

Globally disable cache #212

Closed
alexbowers-tecmark opened this issue Mar 4, 2015 · 16 comments
Closed

Globally disable cache #212

alexbowers-tecmark opened this issue Mar 4, 2015 · 16 comments

Comments

@alexbowers-tecmark
Copy link

I am looking for a way to globally disable cache for a pool, for use during development.

I was hoping something like $pool->disable() would exist, which would then simply force isMiss() to return true, so that the operation happens always.

This would mean that regardless of the timeout time, it would be ignored.

@tedivm
Copy link
Member

tedivm commented Mar 4, 2015

The foundation for this feature is already here- there's a pool disable feature that exists and gets activated in the case of certain errors, and it's controlled by the "isDisabled" property-

https://github.com/tedious/Stash/blob/master/src/Stash/Pool.php#L35

If you wanted to make a pull request to expose that via a "disable" function I would merge it.

That being said, my advice for development is to use the "ephemeral" driver. It stores things only for the current request, and once that php process or request is finished the data goes away with it. This driver is the default driver if you don't pass one in.

@alexbowers
Copy link
Contributor

We would also have the use case for having an admin panel, where the customer (read: admin user of service) can enable or disable caching on API calls themselves (for whatever reason).

I'll give a PR a quick go.

@alexbowers
Copy link
Contributor

#214

Pull request made.

@alexbowers-tecmark
Copy link
Author

A note I should make is that I couldn't use the isDisabled property for this, since that would only return null, which isn't what the desired effect was. The desired effect was to force the cache to miss, effectively meaning that the cache is no longer there (Eg. doing $item->set($response) ... $item->get() with a forced miss a round the set would be the same as return $response)

@alexbowers
Copy link
Contributor

any news on a merge?

@tedivm
Copy link
Member

tedivm commented Mar 9, 2015

It would only return null where? I'm confused as to what was wrong with using isDisable.

@alexbowers-tecmark
Copy link
Author

When using isDisable(), isMiss() would return null. This is unexpected, because the feature is that isMiss() would be forced, so that the cache is effectively bypassed.

@alexbowers
Copy link
Contributor

I was incorrect on this, it isn't isMiss() that returns null, it is get() that returns null. Explained in PR.

isDisabled will force the cache to be bypassed, but the stored value cannot be fetched, so it is effectively useless for this use case.

@tedivm
Copy link
Member

tedivm commented Mar 11, 2015

So you want a read only cache?

@alexbowers-tecmark
Copy link
Author

A read only cache, but that still writes to the cache so that when the site comes out of dev, the cache doesn't have to be regenerated for the items that are correct

@tedivm
Copy link
Member

tedivm commented Mar 25, 2015

A read-only cache is one that you can read items from but not put items into it (you can "read" the cache but not "write" the cache). What you want is the opposite- a write only cache that doesn't allow reading.

Lets be clear about this in the API. I'm not going to approve this as "disable" because that's not what it does. There are three states here that each need to be defined-

  • Ephemeral - per request caching only, with no persistent storage.
  • Disable- turns the caching system off completely.
  • Read Only- can pull items from the cache but new or replacement items can't be added.
  • Write Only- can fill the cache but does not read items from it.

A "setMode" function that takes these options seems like the way to go.

@alexbowers-tecmark
Copy link
Author

If an appropriate name was found, would you accept this as a renamed method?

Eg. writeOnly(), forceMiss() or something along those lines?

Or do you mean that we would have some constants defined, and that as well as ->setDriver() we would also have a setMode() which would take the value of one of the four bullet points?

@tedivm
Copy link
Member

tedivm commented Mar 25, 2015

I'd prefer the setMode method, although I think it should skip the Ephemeral option (that can work using the ephemeral driver as is). So the options would be-

  • Normal
  • Read Only
  • Write Only
  • Disabled

@alexbowers
Copy link
Contributor

OK I'll give be this a go tomorrow if you haven't started. Want me to
modify my PR or start fresh?
On 25 Mar 2015 20:20, "Robert Hafner" notifications@github.com wrote:

I'd prefer the setMode method, although I think it should skip the
Ephemeral option (that can work using the ephemeral driver as is). So the
options would be-

  • Normal
  • Read Only
  • Write Only
  • Disabled


Reply to this email directly or view it on GitHub
#212 (comment).

@tedivm
Copy link
Member

tedivm commented Mar 25, 2015

Starting fresh might make the most sense.

@alexbowers
Copy link
Contributor

I'll create a new issue for this then, so we can discuss it before I start work.

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

No branches or pull requests

3 participants