[Improvement] Wire the Pimcore cache pool to the Valkey container by berfinyuksel · Pull Request #297 · pimcore/skeleton

berfinyuksel · 2026-10-13T11:51:55Z

Why

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). Core's CacheFallbackPass picks the DB adapter whenever no pool is defined; nothing reads REDIS_URL on 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

File	Change
`docker-compose.yaml`	`redis` → `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 128mb` as a single argument.
`.env`	`REDIS_URL=redis://valkey:6379` (same as demo-enterprise)
`config/config.yaml`	`pimcore.cache.pool` enabled with `provider: '%env(REDIS_URL)%'`
`config/packages/test/config.yaml`	`cache.adapter.array` for tests (same as pimcore/pimcore's CI test config): tests neither depend on the cache container nor share entries with the dev instance
`src/Installer/SkeletonProfile.php`	`RedisEnvVarDefinition` so the installer validates the cache connection
`.github/workflows/pimcore-skeleton.yml`	wait for `valkey:6379` before running the installer; `--skip-validation=redis` for the test installation, since the test env uses the array adapter and `docker compose run test-php` does not start Valkey
`README.md`	mention Valkey and that the cache is pre-wired

Verification

YAML of all edited files parses; RedisEnvVarDefinition exists in v2026.2.14+ and on 2026.x.
provider: '%env(REDIS_URL)%' with redis_tag_aware is what paas-skeleton runs in production.
Not run locally (no Docker on my machine): the CI workflow does the full compose up → install → Codeception run and is the runtime check.
New CI step "Verify the Pimcore cache is served by Valkey": runs pimcore:cache:warming after the install (forced writes go through RedisTagAwareAdapter::doSave, including its eviction-policy check) and asserts valkey-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

With the pool enabled, REDIS_URL becomes mandatory wherever config.yaml is 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.
Sessions are out of scope (still PdoSessionHandler).

🤖 Generated with Claude Code

Part of pimcore/DevOps-Tasks#100

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>

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>

Copilot

🟡 Changes recommended

Test installation still validates Valkey connectivity despite using an in-memory cache.

1 open finding

Test installation fails without Valkey due to Redis validation · New

What changed in this PR

Wires Pimcore’s object cache to Valkey instead of the database fallback.

Changes:

Enables the cache pool through REDIS_URL and 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.

berfinyuksel · 2026-10-28T10:39:23Z

            new AmqpMessengerEnvVarDefinition(),
            new MercureEnvVarDefinition(),
            new ProductRegistrationEnvVarDefinition(),
+            new RedisEnvVarDefinition(),


Confirmed and fixed in a0446fe: added --skip-validation=redis to the test-install line, in line with the other services the test environment does not use (opensearch, messenger-amqp, mercure). The dev install still validates the Valkey connection, which is the point of adding the definition.

mcop1 · 2026-10-28T10:16:32Z

Have a look at the copilot finding and fix it, if necessary, Other than that it lgtm.

The test environment uses cache.adapter.array, so the installer must not require a reachable Valkey there. The committed REDIS_URL activates the optional RedisEnvVarDefinition, whose validator opens a real connection regardless of APP_ENV, and `docker compose run test-php` does not start the valkey service. Skip it like the other services the test env does not use. Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

berfinyuksel · 2026-10-28T10:47:22Z

CI note: the CliCommandTest failure on a0446fe is unrelated to this PR. pimcore/pimcore#19596 (2026-10-21) allows DoctrineBundle 3, which now rejects the host/port/dbname/user/password: ~ lines next to url in config/packages/test/config.yaml (same cause as pimcore/platform-version#673). Fix on 2026.x in #299; once that is merged I will merge 2026.x into this branch to get it green.

Copilot

🟡 Changes recommended

The --save value in docker-compose.yaml passes literal quote characters instead of an empty string, causing Valkey to exit and blocking installation and the CI readiness check.

2 open issues

_{Review effort: Balanced}

berfinyuksel · 2026-10-28T12:01:30Z

+        command: [ valkey-server, --maxmemory, 128mb, --maxmemory-policy, volatile-lru, --save, '""' ]
+
+    # Valkey is a drop-in replacement for Redis. Comment out the valkey service
+    # above and uncomment the one below to use Redis instead.
+#    redis:
+#        image: redis:alpine
+#        command: [ redis-server, --maxmemory, 128mb, --maxmemory-policy, volatile-lru, --save, '""' ]


Checked against the Valkey source and the CI run; this is not an issue, no change needed.

The YAML '""' does pass the two-character string "" to valkey-server, but Valkey accepts it:

server.c main() appends option arguments via sdscatrepr, so the config text becomes save "\"\"".

config.c loadServerConfigFromString() splits that line to argv = [save, ""] (the two-char value).

save is a MULTI_ARG_CONFIG with exactly one non-empty argument, so the loader re-splits the value with sdssplitargs("\"\"") → one empty token (config.c, "For MULTI_ARG_CONFIGs, if we only have one argument, try to split it by spaces").

setConfigSaveOption() then hits its special case argc == 1 && argv[0] == "" → resetServerSaveParams(), i.e. snapshots disabled. That is the same path a save "" line in valkey.conf takes.

Empirically: the green run on this PR (https://github.com/pimcore/skeleton/actions/runs/40677157135) started Valkey with exactly this command, dockerize -wait tcp://valkey:6379 succeeded, and the new CI step reported Keys in Valkey after cache warming: 7. demo-enterprise has run the identical line since July 2026 (pimcore/demo-enterprise#524). An empty '' would also work, but '""' is kept for consistency with demo-enterprise and the conf-file idiom.

berfinyuksel requested a review from bluvulture as a code owner October 13, 2026 11:51

berfinyuksel requested a review from mcop1 October 13, 2026 11:52

berfinyuksel added the Skip Milestone Check Exempts this PR from the mandatory milestone requirement label Oct 15, 2026

berfinyuksel and others added 2 commits October 15, 2026 12:19

bluvulture requested a balanced review from Copilot October 15, 2026 10:43

Copilot started reviewing on behalf of bluvulture October 15, 2026 10:44

View session

Copilot AI reviewed Oct 15, 2026

View reviewed changes

mcop1 approved these changes Oct 28, 2026

View reviewed changes

berfinyuksel mentioned this pull request Oct 28, 2026

[Bug] Remove null connection overrides next to url in the test config (DoctrineBundle 3) #299

Merged

Merge remote-tracking branch 'origin/2026.x' into wire-valkey-cache

455411a

berfinyuksel requested a balanced review from Copilot October 28, 2026 11:09

Copilot started reviewing on behalf of berfinyuksel October 28, 2026 11:09

View session

Copilot AI reviewed Oct 28, 2026

View reviewed changes

berfinyuksel merged commit 4c19e96 into 2026.x Oct 28, 2026
7 checks passed

berfinyuksel deleted the wire-valkey-cache branch October 28, 2026 11:56

berfinyuksel restored the wire-valkey-cache branch October 28, 2026 11:56

github-actions Bot locked and limited conversation to collaborators Oct 28, 2026

berfinyuksel deleted the wire-valkey-cache branch October 28, 2026 12:05

Navigation Menu

Repository navigation

[Improvement] Wire the Pimcore cache pool to the Valkey container - #297

[Improvement] Wire the Pimcore cache pool to the Valkey container#297
berfinyuksel merged 5 commits into
2026.xfrom
wire-valkey-cache

berfinyuksel commented Oct 13, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

berfinyuksel Oct 28, 2026

Uh oh!

mcop1 commented Oct 28, 2026

Uh oh!

berfinyuksel commented Oct 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

berfinyuksel Oct 28, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

berfinyuksel commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Changes

Verification

Reviewer notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

berfinyuksel Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

mcop1 commented Oct 28, 2026

Uh oh!

berfinyuksel commented Oct 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

berfinyuksel Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

berfinyuksel commented Oct 13, 2026 •

edited

Loading