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

Add default TTL #4742

Merged
merged 1 commit into from
May 27, 2021
Merged

Add default TTL #4742

merged 1 commit into from
May 27, 2021

Conversation

MGatner
Copy link
Member

@MGatner MGatner commented May 25, 2021

Description
I am constantly adding a default Cache Time To Live value to modules and projects that interact with the cache. It's quite frustrating that the framework interface and handlers have this value hard-coded, so we cannot change that in version 4, but supplying this in the config file gives an opportunity for developers to implement it themselves (and to replace the hard-coded interface in version 5).

Checklist:

  • Securely signed commits
  • Component(s) with PHPdocs
  • n/a Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

Copy link
Member

@paulbalandan paulbalandan left a comment

Choose a reason for hiding this comment

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

If this is not used by the framework, then how do projects benefit from this?

@MGatner
Copy link
Member Author

MGatner commented May 26, 2021

Two points...

  1. Numerous components rely on the cache. For example, if I want my WidgetFactory to be able to store/save a Widget to cache I can call cache()->save('widget', $widget) but then developers using my WidgetFactory are locked into 60 seconds (which sometimes is wildly inappropriate). If I want to let developers set their own TTL I either need to add parameters to every method that might save to cache or supply a configuration (I typically choose the latter). But then when you make a FruitFactory now we have WidgetConfig::$cacheTTL and FruitConfig::$cacheTTL. If we don't provide a means of managing this in the framework then every component that uses cache is stuck with this choice.
  2. Introducing the configuration value now lets people start using it so there are no surprises when it is added in version 5. This includes us - we cannot change the Cache handler interface but anywhere in the framework we want to pass in $cacheConfig->ttl (with default value 60) we will be maintaining backwards-compatibility while also offering configurability.

@MGatner
Copy link
Member Author

MGatner commented May 26, 2021

I'll also add, TTL is starting to push me towards the cache edge. The handling is very inconsistent (e.g. sometimes 0 means "don't cache", others it means "cache indefinitely"). We've already had some small breaking changes to Cache to fix bugs, and still haven't released our PSR adapters (codeigniter4/cache#2). We might need to think about some larger Cache changes soon.

@MGatner MGatner merged commit 1b5f05e into codeigniter4:develop May 27, 2021
@MGatner MGatner deleted the ttl branch May 27, 2021 12:22
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 this pull request may close these issues.

2 participants