Repository navigation
[Improvement] Wire the Pimcore cache pool to the Valkey container - #297
Open
berfinyuksel wants to merge 3 commits into
Open
berfinyuksel wants to merge 3 commits into
berfinyuksel wants to merge 3 commits into
Conversation
docker-compose.yaml started a Redis container, but nothing used it: the pimcore.cache.pool block in config/config.yaml was commented out, so a fresh install silently ran Pimcore's object cache on the database adapter (cache_items table). - docker-compose.yaml: replace the redis service with valkey (same image, command and commented-out Redis alternative as demo-enterprise) so both packages use the same cache service. - .env: add REDIS_URL=redis://valkey:6379, like demo-enterprise. - config/config.yaml: enable the pimcore.cache.pool block with provider '%env(REDIS_URL)%'. - config/packages/test/config.yaml: use cache.adapter.array for tests (same as pimcore/pimcore's CI test config), so tests neither depend on the cache service nor share entries with the dev instance. - SkeletonProfile: add RedisEnvVarDefinition so the installer validates the cache connection during installation. - CI workflow: wait for valkey:6379 before running the installer. - README: mention Valkey and that the cache is pre-wired. Co-Authored-By: Claude <noreply@anthropic.com>
berfinyuksel
added
the
Skip Milestone Check
CI failed in "Run codeception tests" with: Service ".pimcore.cache.pool.inner": The argument "1" doesn't exist in class "Symfony\Component\Cache\Adapter\ArrayAdapter". Symfony merges pool options per pool name across config files, so in the test environment the array-adapter override still carried the provider '%env(REDIS_URL)%' from config/config.yaml. CachePoolPass maps provider to constructor argument 0 and default_lifetime to argument 1, but cache.adapter.array declares only one argument. Setting provider to null makes both FrameworkExtension and CachePoolPass treat the pool as having no provider (they check with isset), which puts default_lifetime back at argument 0, the same shape as pimcore/pimcore's CI test config. Co-Authored-By: Claude <noreply@anthropic.com>
The existing workflow proves the pool compiles, the installer can connect, and (via demo-enterprise) that reads work, but no step ever wrote to the cache: CLI processes do not write the object cache by default, and a web request flushes its save queue after the response is sent. pimcore:cache:warming uses forced writes, which bypass the CLI gate and go through RedisTagAwareAdapter::doSave, including its eviction-policy check. A fresh install has a root document, asset folder and object folder, so after warming, Valkey must hold keys; the step asserts DBSIZE > 0. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Test installation still validates Valkey connectivity despite using an in-memory cache.
1 open finding
What changed in this PR
Wires Pimcore’s object cache to Valkey instead of the database fallback.
Changes:
- Enables the cache pool through
REDIS_URLand adds installer validation. - Uses an in-memory cache for tests.
- Adds CI readiness and cache-write checks, plus setup documentation.
| File | Description |
|---|---|
src/Installer/SkeletonProfile.php |
Adds Redis connection validation. |
README.md |
Documents the preconfigured Valkey cache. |
docker-compose.yaml |
Replaces Redis with Valkey and fixes command arguments. |
config/packages/test/config.yaml |
Configures an isolated in-memory cache. |
config/config.yaml |
Enables the Redis-compatible cache pool. |
.github/workflows/pimcore-skeleton.yml |
Waits for Valkey and verifies cache writes. |
.env |
Sets the Valkey connection URL. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| new AmqpMessengerEnvVarDefinition(), | ||
| new MercureEnvVarDefinition(), | ||
| new ProductRegistrationEnvVarDefinition(), | ||
| new RedisEnvVarDefinition(), |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
docker-compose.yamlstarted a Redis container, but nothing used it: thepimcore.cache.poolblock inconfig/config.yamlwas commented out, so a fresh install silently ran Pimcore's object cache on the database adapter (cache_itemstable). Core'sCacheFallbackPasspicks the DB adapter whenever no pool is defined; nothing readsREDIS_URLon its own.Follow-up to the discussion on pimcore/platform-version#615: fix it at the source instead of documenting the gap. Companion PR for demo-enterprise: pimcore/demo-enterprise#549.
Changes
docker-compose.yamlredis→valkey(valkey/valkey:alpine), same command and commented-out Redis alternative as demo-enterprise, so both packages use the same cache service. Also fixes the old command, which passed--maxmemory 128mbas a single argument..envREDIS_URL=redis://valkey:6379(same as demo-enterprise)config/config.yamlpimcore.cache.poolenabled withprovider: '%env(REDIS_URL)%'config/packages/test/config.yamlcache.adapter.arrayfor tests (same as pimcore/pimcore's CI test config): tests neither depend on the cache container nor share entries with the dev instancesrc/Installer/SkeletonProfile.phpRedisEnvVarDefinitionso the installer validates the cache connection.github/workflows/pimcore-skeleton.ymlvalkey:6379before running the installerREADME.mdVerification
RedisEnvVarDefinitionexists in v2026.2.14+ and on 2026.x.provider: '%env(REDIS_URL)%'withredis_tag_awareis whatpaas-skeletonruns in production.compose up→ install → Codeception run and is the runtime check.pimcore:cache:warmingafter the install (forced writes go throughRedisTagAwareAdapter::doSave, including its eviction-policy check) and assertsvalkey-cli DBSIZE> 0. Without it no CI step ever writes to the cache: CLI processes don't write the object cache by default, and a web request flushes its save queue after the response is sent.Reviewer notes
REDIS_URLbecomes mandatory whereverconfig.yamlis loaded: a project without a reachable Redis/Valkey now fails to boot instead of silently using the DB. Intended, but a behaviour change for anyone copying the template to a non-Docker host.PdoSessionHandler).🤖 Generated with Claude Code