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

fix(adapter): use set and setex on rediscacheitempooladapter save #60

Merged

Conversation

kronostof
Copy link
Contributor

@kronostof kronostof commented Sep 30, 2021

Why

RedisCacheItemPoolAdapter switch from set declaration on v3.0.0 to set declaration on v6.0.1

how

Using set and setex as it should be

@kronostof kronostof force-pushed the fix/redis-cache-item-pool-adapter branch from 4090889 to 7d1526f Compare September 30, 2021 15:07
@kronostof kronostof force-pushed the fix/redis-cache-item-pool-adapter branch 2 times, most recently from 43a0882 to 2e217e2 Compare October 8, 2021 14:38
@e-zannelli e-zannelli self-requested a review October 8, 2021 15:43
->isInstanceof('Psr\Cache\CacheItemInterface')
->string($item->get())
->isEqualTo('myExpirableValue')
->if(sleep(1))
Copy link
Contributor

Choose a reason for hiding this comment

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

you could use usleep for waiting less time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could we talk about this point ?
As we use redis-mock i could not check that we the method set with the right argument was call.
As clientItem use millisecond format for time expiration (and for some unknown reasons), using shorter time range than half second introduce some wired false-positive 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Has we saw together there is an issue on RedisMock-bundle. Ttl resolution by milliseconde could not be use.

@kronostof kronostof force-pushed the fix/redis-cache-item-pool-adapter branch 3 times, most recently from bbad48e to 80296c7 Compare October 18, 2021 12:33
@kronostof kronostof force-pushed the fix/redis-cache-item-pool-adapter branch from 80296c7 to b046946 Compare October 25, 2021 13:25
fix(adapter): replace set by set with option on rediscacheitempooladapter
@kronostof kronostof force-pushed the fix/redis-cache-item-pool-adapter branch from b046946 to 75eed59 Compare October 25, 2021 15:32
@kronostof kronostof merged commit 627eaec into BedrockStreaming:master Oct 27, 2021
@kronostof kronostof deleted the fix/redis-cache-item-pool-adapter branch October 27, 2021 07:53
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.

5 participants