Skip to content

Fix Redis expiry (#616)#695

Closed
AnTheMaker wants to merge 1 commit intosimplepie:masterfrom
AnTheMaker:patch-1
Closed

Fix Redis expiry (#616)#695
AnTheMaker wants to merge 1 commit intosimplepie:masterfrom
AnTheMaker:patch-1

Conversation

@AnTheMaker
Copy link

This should fix a bug in the Redis cache-handler which caused the timout and prefix paramters to be ignored. Because of that you couldn't set a custom prefix for the Redis keys and all Redis data was stored indefinitely, without an expiry/timeout (Issue #616).

The correct way to use the Redis cache-handler (according to the code comments and documentation) is the following:
redis://localhost:6379/?timeout=3600&prefix=sp_

Until now however, the timout and prefix paramters were ignored. The Parameters already got parsed, so I just added two IFs to check if they are set and use them (or fall back to the defaults which were already there in the code if they aren't set).

If there are any questions, please let me know!
I hope this gets merged (or fixed differently) as I'd love to use the Redis cache-handler instead of the flat-file cache but I can't use it because of the missing expiry functionality (at least until now)!

@mblaney
Copy link
Member

mblaney commented Sep 17, 2021

Hi @AnTheMaker you're right that those url parameters are being ignored, but passing prefix and expire in as the $options parameter should work? As it is your patch breaks that use by assigning an empty array to $options.

@Art4
Copy link
Contributor

Art4 commented Jan 27, 2023

Hey @AnTheMaker Since SimplePie 1.8.0 the SimplePie\Cache\Redis class is deprecated. You can workaround your issue by providing the Redis cache as PSR-16, e.g. using this adapter: https://github.com/php-cache/redis-adapter

@mblaney I think, this PR can be closed.

@AnTheMaker
Copy link
Author

Alright, thanks a lot for the info @Art4!

@AnTheMaker AnTheMaker closed this Jan 29, 2023
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.

3 participants