Skip to content

[Cache] Redis Tag Aware warn on wrong eviction policy #34403

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

Conversation

andrerom
Copy link
Contributor

@andrerom andrerom commented Nov 15, 2019

Q A
Branch? 4.4
Bug fix? yes
Deprecations? no
Tickets n.a.
License MIT
Doc PR n.a.

Adds validation to make sure Redis has been setup with the supported eviction policy to avoid surprises when cache suddenly is inconsistent.

This PR replaces #34178, and instead of checking in constructor and throwing it only checks on save, warns about this and refuses to save cache as suggested on the other PR.

TODO:

  • Adapt test setups for this to set correct eviction policy It already uses default noeviction

@andrerom andrerom changed the title Redis tag aware warn on wrong eviction policy [Cache] Redis Tag Aware warn on wrong eviction policy Nov 15, 2019
@andrerom andrerom force-pushed the redisTagAware_warn_on_wrong_eviction_policy branch from 09972f9 to 304ea8d Compare November 16, 2019 00:21
@nicolas-grekas nicolas-grekas added this to the 4.4 milestone Nov 16, 2019
@nicolas-grekas nicolas-grekas force-pushed the redisTagAware_warn_on_wrong_eviction_policy branch from 304ea8d to e77f6de Compare November 16, 2019 08:54
@nicolas-grekas
Copy link
Member

Thank you @andrerom.

nicolas-grekas added a commit that referenced this pull request Nov 16, 2019
…rerom)

This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

[Cache] Redis Tag Aware warn on wrong eviction policy

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| Deprecations? | no
| Tickets       | n.a.
| License       | MIT
| Doc PR        | n.a.

Adds validation to make sure Redis has been setup with the supported eviction policy to avoid surprises when cache suddenly is inconsistent.

This PR replaces #34178, and instead of checking in constructor and throwing it only checks on save, warns about this and refuses to save cache as suggested on the other PR.

TODO:
- [x] ~Adapt test setups for this to set correct eviction policy~ _It already uses default noeviction_

Commits
-------

e77f6de [Cache] Redis Tag Aware warn on wrong eviction policy
@nicolas-grekas nicolas-grekas merged commit e77f6de into symfony:4.4 Nov 16, 2019
This was referenced Nov 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants