Skip to content

Actually set the TTL on redis set - #24796

Merged
rullzer merged 1 commit into
masterfrom
fix/set_ttl_on_set_redis
Dec 21, 2020
Merged

Actually set the TTL on redis set#24796
rullzer merged 1 commit into
masterfrom
fix/set_ttl_on_set_redis

Conversation

@rullzer

Copy link
Copy Markdown
Member

Else well the keys remain for ever and ever.

Signed-off-by: Roeland Jago Douma roeland@famdouma.nl

@rullzer

Copy link
Copy Markdown
MemberAuthor

/backport to stable20

@rullzer

Copy link
Copy Markdown
MemberAuthor

/backport to stable19

@rullzer

Copy link
Copy Markdown
MemberAuthor

/backport to stable18

@PVince81

Copy link
Copy Markdown
Member

was wondering why we can't use setex as well, found this but not sure: https://www.reddit.com/r/redis/comments/8suupj/expire_option_for_setnx_command/

maybe PHP doesn't have a mapping

@PVince81

Copy link
Copy Markdown
Member

from the docs:

// Will set the key, if it doesn't exist, with a ttl of 10 seconds
$redis->set('key', 'value', ['nx', 'ex'=>10]);

see https://github.com/phpredis/phpredis

@rullzer

This comment has been minimized.

@rullzer

Copy link
Copy Markdown
MemberAuthor

from the docs:

// Will set the key, if it doesn't exist, with a ttl of 10 seconds
$redis->set('key', 'value', ['nx', 'ex'=>10]);

see https://github.com/phpredis/phpredis

aaah that would be fine as well of course

@rullzer

Copy link
Copy Markdown
MemberAuthor

See the fixup for review

@PVince81PVince81 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looks fine, not tested 👍

Else well the keys remain for ever and ever.
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer
rullzerforce-pushed the fix/set_ttl_on_set_redis branch from 1526a8b to 4f0fae8CompareDecember 21, 2020 12:33
@ChristophWurstChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Dec 21, 2020
@faily-bot

faily-botBot commented Dec 21, 2020

Copy link
Copy Markdown

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 615: failure

mysql8.0-php7.4

Show full log
There were 2 warnings:
1) Test\Files\ViewTest::testRenameFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static
2) Test\Files\ViewTest::testCopyFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static
--
There was 1 failure:
1) Test\Files\Cache\MoveFromCacheTraitTest::testLongId
null does not match expected type "array".
/drone/src/tests/lib/Files/Cache/CacheTest.php:521

@rullzer
rullzer merged commit 5579b1d into masterDec 21, 2020
@rullzer
rullzer deleted the fix/set_ttl_on_set_redis branch December 21, 2020 13:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to releaseReady to be released and/or waiting for tests to finishbug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@rullzer@PVince81@ChristophWurst@juliusknorr