Use psr/cache for result caching
#4620
Conversation
derrabus
force-pushed
the
improvement/psr-cache
branch
2 times, most recently
from
a8657a0
to
45f19e0
Compare
derrabus
force-pushed
the
improvement/psr-cache
branch
from
45f19e0
to
256da03
Compare
|
I've addressed your comments. No idea why the OCI8 jobs are failing suddenly, but the failures look unrelated. |
|
Might be related to https://twitter.com/meShivamMathur/status/1386942165272567815 |
|
Reported at shivammathur/setup-php#449 |
|
Given that ORM 2.9 won't support DBAL 3.x, are there any chances of backporting this into 2.x? |
It probably won't be hard to do this, but do we need to? Are DBAL and ORM sharing any caches? |
derrabus
force-pushed
the
improvement/psr-cache
branch
from
256da03
to
cbe01cd
Compare
Depending on the timeline for DBAL 3 support in ORM 2, we don't need to. If that takes a while, I'd be in favour of backporting. |
|
On a different note, with DBAL 3 supporting PHP 7.3+ but ORM 2 supporting 7.1+, there will be a number of users using ORM 2 that can't use DBAL 3. This will slow down the process of getting everybody off doctrine/cache 1.x. |
derrabus
force-pushed
the
improvement/psr-cache
branch
3 times, most recently
from
22ffbc6
to
aefa1a9
Compare
derrabus
changed the title
psr/cache for result caching
derrabus
force-pushed
the
improvement/psr-cache
branch
from
aefa1a9
to
13150f8
Compare
|
Follow-up for the deprecation warnings: #4624 |
I don't have a religion on this, since it's probably only going to have an effect on the dev env. It would make sense to keep them so that people can already notice they should migrate something. @alcaeus WDYT? |
|
If we deprecate them, we should also emit deprecation notices. I'm fine with deferring the deprecation itself to a future release. |
|
What version is it planned to be released in? There's currently no |
|
Oh right 3.2.x does not even exist at this point, let me fix that. |
Right. I've targeted the 3.1.x branch because there was no 3.2.x yet. This change definitely deserves a minor version bump. |
|
Thanks @derrabus ! |
| if ($resultCache instanceof CacheItemPoolInterface) { | ||
| $this->resultCache = $resultCache; | ||
| } elseif ($resultCache instanceof Cache) { | ||
| $this->resultCache = CacheAdapter::wrap($resultCache); |
There was a problem hiding this comment.
@derrabus my team just stumbled upon this, this is a BC Break I'm afraid, if the cache key contains reserved characters.
There was a problem hiding this comment.
Can you elaborate? Or better, can you open an issue so we can discuss the BC break an possible solutions?
There was a problem hiding this comment.
This might be fixed by using rawurlencode instead of throwing here: https://github.com/doctrine/cache/blob/331b4d5dbaeab3827976273e9356b3b453c300ce/lib/Doctrine/Common/Cache/Psr6/CacheAdapter.php#L265 in https://github.com/symfony/symfony/blob/0ac7aa230fe0177214415ee20fec113a44a261f1/src/Symfony/Component/Cache/DoctrineProvider.php#L60
There was a problem hiding this comment.
@derrabus sorry, didn't notice your message. I will open an issue.
Summary
The Doctrine Cache library has served us well, but it's currently being phased out. This PR proposes to rely on PSR-6 for caches. Symfony Cache is used for PHPUnit tests, but PSR-6 should allow us to plug in any application's favorite caching solution easily.