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

[Cache] Fix proxy no expiration to the Redis #44577

Merged
merged 1 commit into from Dec 16, 2021

Conversation

sbelyshkin
Copy link
Contributor

@sbelyshkin sbelyshkin commented Dec 12, 2021

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR

After #38156 RedisAdapter is not able to persist items with no expiration which come through ProxyAdapter because Redis does not persist keys with very large TTL such as \PHP_INT_MAX (64 bits).

@carsonbot carsonbot added this to the 4.4 milestone Dec 12, 2021
@sbelyshkin sbelyshkin force-pushed the fix-proxy-no-expiration branch 3 times, most recently from cd6cb2b to 65bf172 Compare Dec 12, 2021
@sbelyshkin
Copy link
Contributor Author

sbelyshkin commented Dec 12, 2021

Looks like UnitTest fails because Redis cannot connect to getenv('REDIS_HOST') while other tests work well with this variable. How can I rewrite the test?

@carsonbot
Copy link

carsonbot commented Dec 13, 2021

Hey!

I think @ajcerezo has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Good catch, here are some comments before accepting the change.

@sbelyshkin sbelyshkin force-pushed the fix-proxy-no-expiration branch 2 times, most recently from 484407c to 8f8af9c Compare Dec 15, 2021
@sbelyshkin sbelyshkin requested a review from nicolas-grekas Dec 15, 2021
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Dec 16, 2021

Thank you @sbelyshkin.

@nicolas-grekas nicolas-grekas merged commit aaa18df into symfony:4.4 Dec 16, 2021
10 of 11 checks passed
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.

None yet

3 participants