Add a Docker Compose Postgres for local development (Vikunja task #4) #7

rob · 2026-08-02T19:59:54Z

rob commented

2026-08-02 19:59:54 +00:00

Implements Vikunja task #4. Provides the local database so developers do not install Postgres by hand.

Stock postgres:18.4-trixie — no PostGIS. The ticket says "optional PostGIS image, pending decision"; that decision was taken and is recorded in ADR-0006. The ticket text is stale and should be corrected.

Correction to this description's original claim

The first version of this PR said the wrong volume path would "silently persist nothing" and called it a silently-ineffective setting. Review disproved the mechanism by building a throwaway compose file mounting the legacy path and running it. It does not silently lose data — the image aborts on the first up:

Error: in 18+, these Docker images are configured to store database data in a
       format which is compatible with "pg_ctlcluster" ...
       Counter to that, there appears to be PostgreSQL data in:
         /var/lib/postgresql/data (unused mount/volume)
Exited (1)

Recording the correction rather than editing it away. One nuance found while fixing it, which is why the file's comment is not simply "it fails loudly": docker compose up -d still prints Container ... Started and exits 0. The failure is loud in the container logs and invisible at the prompt. The comment now states both halves.

The volume path

Mounts /var/lib/postgresql, not /var/lib/postgresql/data. Postgres 18 sets PGDATA=/var/lib/postgresql/18/docker and declares its VOLUME one level up; the pre-18 convention that most tutorials still show is wrong for this image. Confirmed by docker image inspect in three independent runs, and the failure mode reproduced twice.

Verification actually performed

docker compose config validates; no restart key; host_ip: 127.0.0.1; volume target /var/lib/postgresql.
Container reaches healthy; select version() returns PostgreSQL 18.4.
Connected over host TCP the way the API will.
Loopback binding proven — ss -ltn shows LISTEN 127.0.0.1:5432; a TCP connect to the LAN address is refused.
Persistence proven across down/up by writing and reading back a row — re-run after every change.
.env override proven — non-default database, user, password and port all took effect.
Healthcheck escaping verified on the running container, not by reading the file: docker inspect shows pg_isready --username=${POSTGRES_USER} expanded in-container.
Cold start including initdb took 5.64s, healthy on the first probe.
Docker state cleaned up after every run; the user's pre-existing containers untouched.

Decisions

Image pinned to major.minor and base (18.4-trixie), checked against Docker Hub rather than training data. Debian rather than Alpine: musl collation differences are a poor rehearsal for a managed production Postgres.

No restart policy. Removed in review. Once anyone runs up without a later down, the repository would claim 127.0.0.1:5432 on every boot whether or not they are working on PlaceMark — the same port clash POSTGRES_PORT exists to escape. on-failure would be actively worse: a misconfigured volume path exits 1, so it would turn one visible dead container into a restart loop burying the error that explains it. The README says nothing auto-starts the container, and why.

Weak default credentials and the loopback bind are one decision, not two. placemark/placemark is only defensible because nothing off the machine can reach it. The usual 5432:5432 would publish a database with a guessable password to every interface.

.env.example deliberately does not state the defaults. Every key is commented out with a non-default illustrative value, so the file cannot go stale when a default changes — drift-proof rather than merely deduplicated. cp .env.example .env verified to be a safe no-op.

Healthcheck kept although nothing depends on it yet: it makes docker compose up --wait block until the server genuinely accepts connections, which is what verified this ticket.

No ADR. ADR-0001 sets the bar at constraining the system's shape. This is local tooling. The one decision with real reach, no PostGIS, is already ADR-0006.

Traps documented, not fixed

Postgres reads POSTGRES_USER/PASSWORD/DB only when initialising an empty data directory. Editing .env afterwards silently does nothing — the container starts healthy on the old credentials. Now documented in both the README and .env.example; the first version of this PR claimed that and was wrong, which review caught.

pg_isready is a liveness probe, not an auth probe. A healthy container is not credential validation.

Notes for other tickets

Task #3 owns the connection string. No appsettings*.json touched here.
Testcontainers version drift: when integration tests arrive, their Postgres image tag must be kept in step with this one, and nothing enforces that.
Pre-existing orphan containers — placemark-placemark-1, placemark-mongo and volume placemark_mongo-data, from an older mongo-based incarnation of this repo. Left untouched. Review confirmed docker compose down -v will not remove them (Compose removes only volumes declared in the file), but --remove-orphans will remove the two containers.
The .gitignore rule is redundant, confirmed: the Visual Studio template's *.env already matches .env because a gitignore * matches the empty string, and .env.example is not matched. Kept anyway on the grounds that a file holding credentials deserves a rule stating its intent, in a vendored template that gets regenerated. Reviewer agreed it is defensible either way.

Implements Vikunja task #4. Provides the local database so developers do not install Postgres by hand. Stock `postgres:18.4-trixie` — **no PostGIS.** The ticket says "optional PostGIS image, pending decision"; that decision was taken and is recorded in [ADR-0006](docs/adr/0006-v1-scope-exclusions.md). The ticket text is stale and should be corrected. ## Correction to this description's original claim The first version of this PR said the wrong volume path would "silently persist nothing" and called it a silently-ineffective setting. **Review disproved the mechanism** by building a throwaway compose file mounting the legacy path and running it. It does not silently lose data — the image aborts on the *first* `up`: ``` Error: in 18+, these Docker images are configured to store database data in a format which is compatible with "pg_ctlcluster" ... Counter to that, there appears to be PostgreSQL data in: /var/lib/postgresql/data (unused mount/volume) Exited (1) ``` Recording the correction rather than editing it away. One nuance found while fixing it, which is why the file's comment is not simply "it fails loudly": **`docker compose up -d` still prints `Container ... Started` and exits 0.** The failure is loud in the container logs and invisible at the prompt. The comment now states both halves. ## The volume path **Mounts `/var/lib/postgresql`, not `/var/lib/postgresql/data`.** Postgres 18 sets `PGDATA=/var/lib/postgresql/18/docker` and declares its `VOLUME` one level up; the pre-18 convention that most tutorials still show is wrong for this image. Confirmed by `docker image inspect` in three independent runs, and the failure mode reproduced twice. ## Verification actually performed - `docker compose config` validates; no `restart` key; `host_ip: 127.0.0.1`; volume target `/var/lib/postgresql`. - Container reaches **healthy**; `select version()` returns PostgreSQL 18.4. - Connected over host TCP the way the API will. - **Loopback binding proven** — `ss -ltn` shows `LISTEN 127.0.0.1:5432`; a TCP connect to the LAN address is refused. - **Persistence proven** across `down`/`up` by writing and reading back a row — re-run after every change. - **`.env` override proven** — non-default database, user, password and port all took effect. - **Healthcheck escaping verified on the running container**, not by reading the file: `docker inspect` shows `pg_isready --username=${POSTGRES_USER}` expanded in-container. - Cold start including `initdb` took 5.64s, healthy on the first probe. - Docker state cleaned up after every run; the user's pre-existing containers untouched. ## Decisions **Image pinned to major.minor *and* base** (`18.4-trixie`), checked against Docker Hub rather than training data. Debian rather than Alpine: musl collation differences are a poor rehearsal for a managed production Postgres. **No restart policy.** Removed in review. Once anyone runs `up` without a later `down`, the repository would claim `127.0.0.1:5432` on every boot whether or not they are working on PlaceMark — the same port clash `POSTGRES_PORT` exists to escape. `on-failure` would be actively worse: a misconfigured volume path exits 1, so it would turn one visible dead container into a restart loop burying the error that explains it. The README says nothing auto-starts the container, and why. **Weak default credentials and the loopback bind are one decision, not two.** `placemark`/`placemark` is only defensible because nothing off the machine can reach it. The usual `5432:5432` would publish a database with a guessable password to every interface. **`.env.example` deliberately does not state the defaults.** Every key is commented out with a non-default illustrative value, so the file cannot go stale when a default changes — drift-proof rather than merely deduplicated. `cp .env.example .env` verified to be a safe no-op. **Healthcheck kept** although nothing depends on it yet: it makes `docker compose up --wait` block until the server genuinely accepts connections, which is what verified this ticket. **No ADR.** [ADR-0001](docs/adr/0001-record-architecture-decisions.md) sets the bar at constraining the system's shape. This is local tooling. The one decision with real reach, no PostGIS, is already ADR-0006. ## Traps documented, not fixed **Postgres reads `POSTGRES_USER`/`PASSWORD`/`DB` only when initialising an empty data directory.** Editing `.env` afterwards silently does nothing — the container starts healthy on the *old* credentials. Now documented in both the README and `.env.example`; the first version of this PR claimed that and was wrong, which review caught. **`pg_isready` is a liveness probe, not an auth probe.** A healthy container is not credential validation. ## Notes for other tickets - **Task #3 owns the connection string.** No `appsettings*.json` touched here. - **Testcontainers version drift:** when integration tests arrive, their Postgres image tag must be kept in step with this one, and nothing enforces that. - **Pre-existing orphan containers** — `placemark-placemark-1`, `placemark-mongo` and volume `placemark_mongo-data`, from an older mongo-based incarnation of this repo. Left untouched. Review confirmed `docker compose down -v` will *not* remove them (Compose removes only volumes declared in the file), but `--remove-orphans` will remove the two containers. - **The `.gitignore` rule is redundant**, confirmed: the Visual Studio template's `*.env` already matches `.env` because a gitignore `*` matches the empty string, and `.env.example` is not matched. Kept anyway on the grounds that a file holding credentials deserves a rule stating its intent, in a vendored template that gets regenerated. Reviewer agreed it is defensible either way.

rob added 1 commit

2026-08-02 19:59:54 +00:00

Add a Docker Compose Postgres for local development

Verify runner / verify (pull_request) Successful in 1s

Details

fa13a0d17b

Provides the local database so developers do not install Postgres by
hand. Stock postgres:18.4-trixie, pinned to minor version and base image;
no PostGIS, per ADR-0006.

The volume mounts /var/lib/postgresql, not the /var/lib/postgresql/data
that pre-18 images used and most tutorials still show. Postgres 18 sets
PGDATA to /var/lib/postgresql/18/docker and declares its volume one level
up, so the conventional path would have mounted a volume that persists
nothing while appearing to work until the first down and up. Verified by
writing a row, cycling the stack and reading it back.

Credentials and the published port default in the compose file so a fresh
clone needs no .env, and are overridable through one for developers who
need different values. The port is bound to 127.0.0.1: the default
password is only defensible because nothing off the machine can reach it.

Vikunja task #4.

rob reviewed

2026-08-02 20:07:16 +00:00

rob left a comment

Verdict: mergeable

Independent review. I re-ran every claim rather than reading them; Docker 29.7.1, Compose v5.3.1. All three acceptance criteria of Vikunja task #4 are met and verified. Nothing blocking. Five non-blocking items below, one of which I would like fixed before merge even though it changes no behaviour.

What I verified myself

The volume path — the central claim holds.

$ docker image inspect postgres:18.4-trixie --format '{{json .Config.Volumes}}'
{"/var/lib/postgresql":{}}
PGDATA=/var/lib/postgresql/18/docker

Persistence proven end to end (run under a throwaway project name so the machine's stale placemark project was never touched): up -d --wait → healthy in 5.64s from an empty volume including initdb → create table review_probe / insert 42 → docker compose down (no -v) → up -d --wait → select id from review_probe returns 42. The named volume resolves to placemark_postgres-data, exactly as the README states.

The failure mode is real, but it is not the failure mode you describe. I built a throwaway compose file identical except for postgres-data:/var/lib/postgresql/data. It does not silently persist nothing and then discard the database on down. It refuses to start at all, first boot, empty volume, exit code 1:

Error: in 18+, these Docker images are configured to store database data in a
       format which is compatible with "pg_ctlcluster" ...
       Counter to that, there appears to be PostgreSQL data in:
         /var/lib/postgresql/data (unused mount/volume)
       The suggested container configuration for 18+ is to place a single mount
       at /var/lib/postgresql ...

So the mount path in this PR is right, and the reason it is right is sound — but the image guards this loudly, and the PR description's framing ("appearing to work perfectly until the first docker compose down silently discarded the database", "silently ineffective rather than loudly broken") is not what happens with postgres:18.4-trixie. See the inline comment on the compose file.

Loopback binding. ss -ltn gives LISTEN 0 4096 127.0.0.1:5432 0.0.0.0:*. I also confirmed a TCP connect to this host's LAN address 192.168.1.2:5432 is refused. Claim holds, and this is the right call given the default password.

Healthcheck escaping is correct — this was worth checking because docker compose config re-escapes it and prints $${POSTGRES_USER}, which looks like a bug. The runtime value on the created container is not escaped:

$ docker inspect ... --format '{{json .Config.Healthcheck.Test}}'
["CMD-SHELL","pg_isready --username=${POSTGRES_USER} --dbname=${POSTGRES_DB}"]

Compose passes it through and the container's shell expands it from the container environment. Correct as written.

start_period: 30s / interval: 10s / retries: 5 is sensible and masks nothing. Measured cold start including initdb was 5.64s and the container went healthy on the first probe with FailingStreak=0, so start_period never delays --wait — a passing probe inside the start period marks healthy immediately. Its only effect is roughly 5x headroom before a slow start is called a failure, and worst-case time-to-unhealthy is 30 + 5x10 = 80s. Fine.

.env override, and the documented trap. With POSTGRES_DB=otherdb, POSTGRES_USER=otheruser, POSTGRES_PASSWORD=s3cret, POSTGRES_PORT=55434: ss shows 127.0.0.1:55434, and psql postgres://otheruser:s3cret@127.0.0.1:55434/otherdb returns otheruser|otherdb. The trap is exactly as documented — after changing the credentials in .env and recreating the container, the new credentials fail (FATAL: password authentication failed for user "changed"), the old ones still work, and the container reports healthy throughout. Your pg_isready-is-not-an-auth-probe caveat is confirmed in the same run.

.gitignore. Both of your claims are true, and the explicit rule is genuinely redundant.

$ git check-ignore -v .env
.gitignore:439:.env	.env                       # the new rule wins only because it is last
$ git check-ignore -v .env.example
(no match)
# isolated repo containing only `*.env`:
.gitignore:1:*.env	.env                       # `*` does match the empty string
.env.example                                   # not matched, no negation needed

I would keep it. The argument that a vendored, regeneratable template should not be the only thing standing between a credentials file and the index is a fair one, and the cost is one line. But it is dead config, so the comment above it should stay as honest as it currently is.

Judgements you asked for

POSTGRES_PORT — acceptable, keep it. It is one line, it is the same mechanism as the credentials, and a pre-existing Postgres on 5432 is the failure a newcomer will actually hit. Rejecting it on scope grounds would be pedantry.

restart: unless-stopped — I would drop it. This is my main design disagreement. It is not merely "starts on every boot whether or not you are working on PlaceMark"; it means this repo permanently claims 127.0.0.1:5432 from boot on any machine that has ever run up without a subsequent down. That is precisely the port-clash class of problem POSTGRES_PORT exists to work around — the PR adds an escape hatch for other people's always-on Postgres while making PlaceMark's Postgres one of them. A development database should come up when asked and stay down otherwise; docker compose up -d is not onerous. If you want survival across a Docker daemon restart within a working session, restart: on-failure gets that without the boot behaviour. Non-blocking, but I think the ticket's own framing ("developers need not install Postgres by hand") argues for a database that behaves less like an installed service, not more.

No ADR — agreed. ADR-0001's bar is not met by local tooling, and the decision with reach (no PostGIS) is already ADR-0006. Nothing here contradicts ADR-0006 or ADR-0007; the compose header pointing at ADR-0007 is a nice touch.

README sufficiency — yes. Someone who has never run this project gets the three commands, the volume name, the full default set, a connection string, both psql routes, how to override, and how to destroy. That is more than most repos manage. British English is clean throughout the added prose.

Non-blocking, in priority order

The compose comment's stated failure mode is wrong (docker-compose.yml:11) — inline. This is the one I would like changed before merge.
The trap is documented in the README only, not in .env.example, contrary to the PR description — inline. It is two lines and it is the file a developer will be looking at when they trip over it.
restart: unless-stopped — inline, argument above.
The default values now live in three places — docker-compose.yml, .env.example and the README table. Nothing keeps them in step, and a drift between them is silent and confusing in exactly the way this PR is otherwise careful about. No fix needed now; worth knowing it is a maintenance edge.
README.md:100 now says the project requires Docker outright, but dotnet build and dotnet test need nothing of the sort today. It will become true when Testcontainers arrives. Trivial, mention only for accuracy.

On the stale `placemark` project, since you raised it

I checked rather than assumed, because down -v is in the README you just wrote. placemark-placemark-1, placemark-mongo and the placemark_mongo-data volume all carry com.docker.compose.project=placemark, so this compose file adopts their project namespace. I simulated the exact shape in a throwaway project: docker compose down -v removes only volumes declared in the compose file — the undeclared-but-labelled volume survived. So the README's down -v cannot destroy placemark_mongo-data; the exposure is limited to --remove-orphans deleting the two containers, which is what your description says. Your note is accurate and appropriately scoped.

Everything I started is torn down: all four test projects down -v, test volumes removed, no Postgres port listening. The three pre-existing artefacts are untouched and still exactly as I found them.

Verdict: mergeable Independent review. I re-ran every claim rather than reading them; Docker 29.7.1, Compose v5.3.1. All three acceptance criteria of Vikunja task #4 are met and verified. Nothing blocking. Five non-blocking items below, one of which I would like fixed before merge even though it changes no behaviour. ## What I verified myself **The volume path — the central claim holds.** ``` $ docker image inspect postgres:18.4-trixie --format '{{json .Config.Volumes}}' {"/var/lib/postgresql":{}} PGDATA=/var/lib/postgresql/18/docker ``` Persistence proven end to end (run under a throwaway project name so the machine's stale `placemark` project was never touched): `up -d --wait` → healthy in 5.64s from an empty volume including `initdb` → `create table review_probe` / `insert 42` → `docker compose down` (no `-v`) → `up -d --wait` → `select id from review_probe` returns `42`. The named volume resolves to `placemark_postgres-data`, exactly as the README states. **The failure mode is real, but it is not the failure mode you describe.** I built a throwaway compose file identical except for `postgres-data:/var/lib/postgresql/data`. It does not silently persist nothing and then discard the database on `down`. It refuses to start at all, first boot, empty volume, exit code 1: ``` Error: in 18+, these Docker images are configured to store database data in a format which is compatible with "pg_ctlcluster" ... Counter to that, there appears to be PostgreSQL data in: /var/lib/postgresql/data (unused mount/volume) The suggested container configuration for 18+ is to place a single mount at /var/lib/postgresql ... ``` So the mount path in this PR is right, and the reason it is right is sound — but the image guards this loudly, and the PR description's framing ("appearing to work perfectly until the first `docker compose down` silently discarded the database", "silently ineffective rather than loudly broken") is not what happens with `postgres:18.4-trixie`. See the inline comment on the compose file. **Loopback binding.** `ss -ltn` gives `LISTEN 0 4096 127.0.0.1:5432 0.0.0.0:*`. I also confirmed a TCP connect to this host's LAN address `192.168.1.2:5432` is refused. Claim holds, and this is the right call given the default password. **Healthcheck escaping is correct** — this was worth checking because `docker compose config` re-escapes it and prints `$${POSTGRES_USER}`, which looks like a bug. The runtime value on the created container is not escaped: ``` $ docker inspect ... --format '{{json .Config.Healthcheck.Test}}' ["CMD-SHELL","pg_isready --username=${POSTGRES_USER} --dbname=${POSTGRES_DB}"] ``` Compose passes it through and the container's shell expands it from the container environment. Correct as written. **`start_period: 30s` / `interval: 10s` / `retries: 5` is sensible and masks nothing.** Measured cold start including `initdb` was 5.64s and the container went healthy on the first probe with `FailingStreak=0`, so `start_period` never delays `--wait` — a passing probe inside the start period marks healthy immediately. Its only effect is roughly 5x headroom before a slow start is called a failure, and worst-case time-to-unhealthy is 30 + 5x10 = 80s. Fine. **`.env` override, and the documented trap.** With `POSTGRES_DB=otherdb`, `POSTGRES_USER=otheruser`, `POSTGRES_PASSWORD=s3cret`, `POSTGRES_PORT=55434`: `ss` shows `127.0.0.1:55434`, and `psql postgres://otheruser:s3cret@127.0.0.1:55434/otherdb` returns `otheruser|otherdb`. The trap is exactly as documented — after changing the credentials in `.env` and recreating the container, the new credentials fail (`FATAL: password authentication failed for user "changed"`), the old ones still work, and the container reports **healthy** throughout. Your `pg_isready`-is-not-an-auth-probe caveat is confirmed in the same run. **`.gitignore`. Both of your claims are true, and the explicit rule is genuinely redundant.** ``` $ git check-ignore -v .env .gitignore:439:.env .env # the new rule wins only because it is last $ git check-ignore -v .env.example (no match) # isolated repo containing only `*.env`: .gitignore:1:*.env .env # `*` does match the empty string .env.example # not matched, no negation needed ``` I would keep it. The argument that a vendored, regeneratable template should not be the only thing standing between a credentials file and the index is a fair one, and the cost is one line. But it is dead config, so the comment above it should stay as honest as it currently is. ## Judgements you asked for **`POSTGRES_PORT` — acceptable, keep it.** It is one line, it is the same mechanism as the credentials, and a pre-existing Postgres on 5432 is the failure a newcomer will actually hit. Rejecting it on scope grounds would be pedantry. **`restart: unless-stopped` — I would drop it.** This is my main design disagreement. It is not merely "starts on every boot whether or not you are working on PlaceMark"; it means this repo permanently claims `127.0.0.1:5432` from boot on any machine that has ever run `up` without a subsequent `down`. That is precisely the port-clash class of problem `POSTGRES_PORT` exists to work around — the PR adds an escape hatch for other people's always-on Postgres while making PlaceMark's Postgres one of them. A development database should come up when asked and stay down otherwise; `docker compose up -d` is not onerous. If you want survival across a Docker daemon restart within a working session, `restart: on-failure` gets that without the boot behaviour. Non-blocking, but I think the ticket's own framing ("developers need not install Postgres by hand") argues for a database that behaves less like an installed service, not more. **No ADR — agreed.** ADR-0001's bar is not met by local tooling, and the decision with reach (no PostGIS) is already ADR-0006. Nothing here contradicts ADR-0006 or ADR-0007; the compose header pointing at ADR-0007 is a nice touch. **README sufficiency — yes.** Someone who has never run this project gets the three commands, the volume name, the full default set, a connection string, both `psql` routes, how to override, and how to destroy. That is more than most repos manage. British English is clean throughout the added prose. ## Non-blocking, in priority order 1. **The compose comment's stated failure mode is wrong** (`docker-compose.yml:11`) — inline. This is the one I would like changed before merge. 2. **The trap is documented in the README only, not in `.env.example`**, contrary to the PR description — inline. It is two lines and it is the file a developer will be looking at when they trip over it. 3. **`restart: unless-stopped`** — inline, argument above. 4. **The default values now live in three places** — `docker-compose.yml`, `.env.example` and the README table. Nothing keeps them in step, and a drift between them is silent and confusing in exactly the way this PR is otherwise careful about. No fix needed now; worth knowing it is a maintenance edge. 5. **`README.md:100`** now says the project requires Docker outright, but `dotnet build` and `dotnet test` need nothing of the sort today. It will become true when Testcontainers arrives. Trivial, mention only for accuracy. ## On the stale `placemark` project, since you raised it I checked rather than assumed, because `down -v` is in the README you just wrote. `placemark-placemark-1`, `placemark-mongo` and the `placemark_mongo-data` volume all carry `com.docker.compose.project=placemark`, so this compose file adopts their project namespace. I simulated the exact shape in a throwaway project: `docker compose down -v` removes **only volumes declared in the compose file** — the undeclared-but-labelled volume survived. So the README's `down -v` cannot destroy `placemark_mongo-data`; the exposure is limited to `--remove-orphans` deleting the two containers, which is what your description says. Your note is accurate and appropriately scoped. Everything I started is torn down: all four test projects `down -v`, test volumes removed, no Postgres port listening. The three pre-existing artefacts are untouched and still exactly as I found them.

.env.example

					
				@ -0,0 +9,4 @@

				#

				# `.env` is gitignored. Never commit it, and never reuse these values anywhere but your machine.

				# Application configuration, including the connection string the API uses, is separate and goes

				# through `dotnet user-secrets` — not this file.

The PR description says the init-only credentials trap is "documented in both the README and .env.example". It is documented in the README only — this comment block covers what the file is for, that it is gitignored, and that application config goes through user-secrets, but not the trap.

That matters because this is the file someone is looking at at the exact moment they trip over it: they edit .env, restart, the container reports healthy, the old credentials still work and the new ones do not. I confirmed all three of those in a run just now.

Two lines here would close it, e.g.:

# Postgres reads these only when it first initialises an empty data directory. Changing them
# afterwards does nothing and the container will still start healthy — `docker compose down -v`
# first, which destroys the existing database.

The PR description says the init-only credentials trap is "documented in both the README and `.env.example`". It is documented in the README only — this comment block covers what the file is for, that it is gitignored, and that application config goes through user-secrets, but not the trap. That matters because this is the file someone is looking at at the exact moment they trip over it: they edit `.env`, restart, the container reports healthy, the old credentials still work and the new ones do not. I confirmed all three of those in a run just now. Two lines here would close it, e.g.: # Postgres reads these only when it first initialises an empty data directory. Changing them # afterwards does nothing and the container will still start healthy — `docker compose down -v` # first, which destroys the existing database.

docker-compose.yml Outdated

					
				@ -0,0 +8,4 @@

				    # Pinned to a specific minor version and base image: an unannounced major bump would silently

				    # change on-disk format, and `latest` makes "works on my machine" unfalsifiable. Postgres 18

				    # keeps its data in $PGDATA=/var/lib/postgresql/18/docker, so the volume goes one level up at

				    # /var/lib/postgresql — the pre-18 /var/lib/postgresql/data path persists nothing here.

The mount path is right and I verified it, but this comment's stated reason is not what the image does.

"the pre-18 /var/lib/postgresql/data path persists nothing here" implies a container that runs and quietly loses data. I tested it: postgres:18.4-trixie refuses to start at all, first boot, empty volume, exit 1 — Error: in 18+, these Docker images are configured to store database data in a format which is compatible with "pg_ctlcluster" ... there appears to be PostgreSQL data in: /var/lib/postgresql/data (unused mount/volume).

This comment exists to stop a future developer "correcting" the path back to the tutorial value, and it will do that job better by telling them the truth: the image detects a mount at the legacy path and aborts, and the mount belongs one level up because $PGDATA is version-scoped at /var/lib/postgresql/18/docker so that pg_upgrade --link does not cross a mount boundary. As written, someone who hits the real error will not recognise it from this comment and may not trust the rest of the file.

The same overstatement runs through the PR description ("appearing to work perfectly until the first docker compose down silently discarded the database") — worth correcting there too, since it is offered as the headline finding.

The mount path is right and I verified it, but this comment's stated reason is not what the image does. "the pre-18 /var/lib/postgresql/data path persists nothing here" implies a container that runs and quietly loses data. I tested it: `postgres:18.4-trixie` refuses to start at all, first boot, empty volume, exit 1 — `Error: in 18+, these Docker images are configured to store database data in a format which is compatible with "pg_ctlcluster" ... there appears to be PostgreSQL data in: /var/lib/postgresql/data (unused mount/volume)`. This comment exists to stop a future developer "correcting" the path back to the tutorial value, and it will do that job better by telling them the truth: the image detects a mount at the legacy path and aborts, and the mount belongs one level up because $PGDATA is version-scoped at /var/lib/postgresql/18/docker so that `pg_upgrade --link` does not cross a mount boundary. As written, someone who hits the real error will not recognise it from this comment and may not trust the rest of the file. The same overstatement runs through the PR description ("appearing to work perfectly until the first `docker compose down` silently discarded the database") — worth correcting there too, since it is offered as the headline finding.

docker-compose.yml Outdated

					
				@ -0,0 +10,4 @@

				    # keeps its data in $PGDATA=/var/lib/postgresql/18/docker, so the volume goes one level up at

				    # /var/lib/postgresql — the pre-18 /var/lib/postgresql/data path persists nothing here.

				    image: postgres:18.4-trixie

				    restart: unless-stopped

I would drop restart: unless-stopped, or use restart: on-failure.

unless-stopped restarts the container when the Docker daemon starts, so once anyone has run docker compose up without a later down, this repo holds 127.0.0.1:5432 from boot, forever, on a machine that may not touch PlaceMark for weeks.

That is the same problem POSTGRES_PORT was added to escape — you have given developers a workaround for other people's always-on Postgres while making this one of them. A development database is better as something that runs when asked. on-failure keeps the useful part (recovery from a crash mid-session) without the boot behaviour.

Not blocking; it is one word either way and reasonable people differ.

I would drop `restart: unless-stopped`, or use `restart: on-failure`. `unless-stopped` restarts the container when the Docker daemon starts, so once anyone has run `docker compose up` without a later `down`, this repo holds 127.0.0.1:5432 from boot, forever, on a machine that may not touch PlaceMark for weeks. That is the same problem `POSTGRES_PORT` was added to escape — you have given developers a workaround for other people's always-on Postgres while making this one of them. A development database is better as something that runs when asked. `on-failure` keeps the useful part (recovery from a crash mid-session) without the boot behaviour. Not blocking; it is one word either way and reasonable people differ.

rob added 1 commit

2026-08-02 20:11:54 +00:00

Action review feedback on the local database

Verify runner / verify (pull_request) Successful in 1s

Details

fcc4fde497

Corrects the failure mode the volume-path comment describes. Review built
a compose file mounting the legacy /var/lib/postgresql/data and ran it:
the image does not silently persist nothing, it aborts on the first up
with an error naming the unused mount. Reproduced independently before
changing the text.

The comment now states both halves, because the distinction matters to
whoever reads it: docker compose up -d still prints Started and exits 0,
so the failure is loud in the container logs and invisible at the prompt.

Removes restart: unless-stopped with no replacement. Once anyone runs up
without a later down, the repository claims 127.0.0.1:5432 on every boot
whether or not they are working on PlaceMark — the same port clash
POSTGRES_PORT exists to escape. on-failure would be worse than nothing
here: a misconfigured volume path exits 1, so it would turn one visible
dead container into a restart loop that buries the explaining error.

Documents in .env.example that Postgres reads these values only when
initialising an empty data directory. The PR description had claimed this
was already there; it was in the README only.

Stops .env.example restating the defaults. Every key is commented out
with a deliberately non-default illustrative value, so the file cannot go
stale when a default changes rather than merely being deduplicated today.

Scopes the Docker prerequisite to the database; building and testing need
nothing but the SDK.

rob reviewed

2026-08-02 20:15:10 +00:00

rob left a comment

Verdict: mergeable

Re-review at fcc4fde, superseding my verdict at fa13a0d. All five findings actioned; I re-ran the compose config, a full persistence cycle, and — because it is now asserted in a code comment — the up -d exit-code claim. Nothing blocking. Two optional nits at the bottom, neither worth another round on its own.

Finding 1 — the new framing is accurate, not a hedge

I checked this specifically because "loud in the logs, invisible at the prompt" is the kind of sentence that sounds true and often is not. It is true, and all three halves hold. Fresh volume, mount moved to /var/lib/postgresql/data, plain docker compose up -d:

 Container failmode2-postgres-1 Creating
 Container failmode2-postgres-1 Created
 Container failmode2-postgres-1 Starting
 Container failmode2-postgres-1 Started
EXIT CODE = 0

Immediately afterwards, docker compose ps still showed Up Less than a second (health: starting) with the port mapping listed. A moment later ps -a gave Exited (1), .State.ExitCode=1, and the unused mount/volume error appeared only in docker compose logs. So there is a real window in which the prompt reports success, the exit code is 0, and ps shows a running container — and the only evidence is in the logs, which nobody reads on a successful-looking up.

That is a better description of the failure than either of ours. Mine ("loudly broken") understated how easy it is to miss; the original ("silently discarded the database") had the wrong mechanism. The comment now says the thing that is both true and useful to the person who is about to "correct" the path. Good.

Finding 3 — the exit-1 argument for dropping `on-failure` is correct

The author's reasoning is sound and I had missed it. Given the container exits 1 rather than hanging, restart: on-failure would loop the entrypoint and flood the log with repeats, burying the single unused mount/volume block that finding 1 exists to make findable. Removing the policy outright is the better call, and it is the same argument as finding 1 rather than a separate one — which is a point in its favour.

Verified on the running container: .HostConfig.RestartPolicy.Name = no. The README's replacement sentence ("Nothing starts the container for you, so run up again after a reboot — deliberate, so that the repository does not hold port 5432 on days you are not using it") is accurate and states the trade-off rather than hiding it, which is better than silently dropping the key.

Finding 4 — better than what I suggested, and I have changed my mind

I asked for deduplication; the author removed the duplicate instead. That is the stronger move and I would not go back to my version.

The distinction that matters: deduplication still leaves two artefacts that must agree, and correctness depends on someone remembering to update both. An .env.example that never states the defaults cannot disagree with them, because it no longer makes a claim about them. # The values below are deliberately *not* the defaults, so that nothing here can quietly fall out of step with docker-compose.yml. They illustrate overriding; they are not a copy of what you get. is exactly right, and the illustrative values are well chosen — placemark_scratch, someone_else, choose-your-own are self-evidently placeholders, so nobody uncomments all four by reflex and then wonders why the README's connection string does not work.

I verified the no-op claim rather than taking it:

$ cp .env.example .env && docker compose config
POSTGRES_DB: placemark
POSTGRES_USER: placemark
POSTGRES_PASSWORD: placemark
host_ip: 127.0.0.1  published: "5432"

No warnings, all four defaults intact, #KEY=value with no space parses as a comment. Uncommenting works: POSTGRES_PASSWORD became choose-your-own and published became "55432".

Is the example less useful to a newcomer? No. Nothing was lost, only moved, and the file says where it went ("those defaults and the connection details they produce are in the README"). A newcomer's actual question at that file is "how do I change the password", which it now answers better than before, because the old version's four default-valued lines invited a copy-and-change-nothing that did nothing.

Is the README's one sentence worse than the five-row table? Marginally different, not worse. The sentence plus the connection string carries every value the table did, and the connection string is the thing anyone actually copies. Dropping the local psql "postgres://..." line in favour of docker compose exec is fine — the exec form works with no host psql installed, and the connection string sits three lines above for anyone who wants to adapt it.

Findings 2 and 5 — confirmed in the files, not taken on trust

The init-only trap is now in .env.example:14-17 in the strongest available form ("the container still starts, still reports healthy, and still holds the old name, username and password"), which matches what I measured last round exactly. The README keeps its own copy at lines 57-58; that duplication is appropriate, since the two files are read at different moments.

README.md:100-101 now scopes the prerequisite correctly, and the [database](#database) anchor resolves to the ### Database heading.

Re-verification of the unchanged parts

Since the compose file changed, I re-ran the lot rather than assuming: docker compose config resolves to project placemark, volume placemark_postgres-data, mount /var/lib/postgresql, port 127.0.0.1:5432; cold up -d --wait reaches healthy; insert 99 → docker compose down (no -v) → up -d --wait → select returns 99; ss -ltn shows LISTEN 127.0.0.1:5432 and nothing on 0.0.0.0. .gitignore is untouched and my earlier check-ignore results still stand.

Optional nits — do not spin another round for these

docker-compose.yml:15-17 — "the container never starts at all" sits a clause away from "reports the container as Started", which is a mild self-contradiction in a comment whose whole job is precision. It does start; it exits 1 within a second. "never comes up" or "exits 1 within a second" would remove the wobble. Genuinely trivial, and the surrounding sentences make the meaning unambiguous.
docker-compose.yml:2 — the header still says "copy .env.example to change them", which was accurate when the keys were live values. It is now copy and uncomment. One word.

Both are a single-word edit if you happen to be touching the file anyway; neither justifies a further review cycle. Merge when ready.

All Docker state I created is gone — three test projects down -v, scratch directories removed, no Postgres port listening. placemark-placemark-1, placemark-mongo and placemark_mongo-data are untouched and exactly as I found them. My worktree is still detached at fa13a0d with a clean tree; I read fcc4fde via git show origin/feat/docker-compose:<path> and never checked it out.

Verdict: mergeable Re-review at `fcc4fde`, superseding my verdict at `fa13a0d`. All five findings actioned; I re-ran the compose config, a full persistence cycle, and — because it is now asserted in a code comment — the `up -d` exit-code claim. Nothing blocking. Two optional nits at the bottom, neither worth another round on its own. ## Finding 1 — the new framing is accurate, not a hedge I checked this specifically because "loud in the logs, invisible at the prompt" is the kind of sentence that sounds true and often is not. It is true, and all three halves hold. Fresh volume, mount moved to `/var/lib/postgresql/data`, plain `docker compose up -d`: ``` Container failmode2-postgres-1 Creating Container failmode2-postgres-1 Created Container failmode2-postgres-1 Starting Container failmode2-postgres-1 Started EXIT CODE = 0 ``` Immediately afterwards, `docker compose ps` still showed `Up Less than a second (health: starting)` with the port mapping listed. A moment later `ps -a` gave `Exited (1)`, `.State.ExitCode=1`, and the `unused mount/volume` error appeared only in `docker compose logs`. So there is a real window in which the prompt reports success, the exit code is 0, and `ps` shows a running container — and the only evidence is in the logs, which nobody reads on a successful-looking `up`. That is a better description of the failure than either of ours. Mine ("loudly broken") understated how easy it is to miss; the original ("silently discarded the database") had the wrong mechanism. The comment now says the thing that is both true and useful to the person who is about to "correct" the path. Good. ## Finding 3 — the exit-1 argument for dropping `on-failure` is correct The author's reasoning is sound and I had missed it. Given the container exits 1 rather than hanging, `restart: on-failure` would loop the entrypoint and flood the log with repeats, burying the single `unused mount/volume` block that finding 1 exists to make findable. Removing the policy outright is the better call, and it is the same argument as finding 1 rather than a separate one — which is a point in its favour. Verified on the running container: `.HostConfig.RestartPolicy.Name = no`. The README's replacement sentence ("Nothing starts the container for you, so run `up` again after a reboot — deliberate, so that the repository does not hold port 5432 on days you are not using it") is accurate and states the trade-off rather than hiding it, which is better than silently dropping the key. ## Finding 4 — better than what I suggested, and I have changed my mind I asked for deduplication; the author removed the duplicate instead. That is the stronger move and I would not go back to my version. The distinction that matters: deduplication still leaves two artefacts that must agree, and correctness depends on someone remembering to update both. An `.env.example` that never states the defaults cannot disagree with them, because it no longer makes a claim about them. `# The values below are deliberately *not* the defaults, so that nothing here can quietly fall out of step with docker-compose.yml. They illustrate overriding; they are not a copy of what you get.` is exactly right, and the illustrative values are well chosen — `placemark_scratch`, `someone_else`, `choose-your-own` are self-evidently placeholders, so nobody uncomments all four by reflex and then wonders why the README's connection string does not work. I verified the no-op claim rather than taking it: ``` $ cp .env.example .env && docker compose config POSTGRES_DB: placemark POSTGRES_USER: placemark POSTGRES_PASSWORD: placemark host_ip: 127.0.0.1 published: "5432" ``` No warnings, all four defaults intact, `#KEY=value` with no space parses as a comment. Uncommenting works: `POSTGRES_PASSWORD` became `choose-your-own` and `published` became `"55432"`. **Is the example less useful to a newcomer?** No. Nothing was lost, only moved, and the file says where it went ("those defaults and the connection details they produce are in the README"). A newcomer's actual question at that file is "how do I change the password", which it now answers better than before, because the old version's four default-valued lines invited a copy-and-change-nothing that did nothing. **Is the README's one sentence worse than the five-row table?** Marginally different, not worse. The sentence plus the connection string carries every value the table did, and the connection string is the thing anyone actually copies. Dropping the local `psql "postgres://..."` line in favour of `docker compose exec` is fine — the `exec` form works with no host psql installed, and the connection string sits three lines above for anyone who wants to adapt it. ## Findings 2 and 5 — confirmed in the files, not taken on trust The init-only trap is now in `.env.example:14-17` in the strongest available form ("the container still starts, still reports healthy, and still holds the old name, username and password"), which matches what I measured last round exactly. The README keeps its own copy at lines 57-58; that duplication is appropriate, since the two files are read at different moments. `README.md:100-101` now scopes the prerequisite correctly, and the `[database](#database)` anchor resolves to the `### Database` heading. ## Re-verification of the unchanged parts Since the compose file changed, I re-ran the lot rather than assuming: `docker compose config` resolves to project `placemark`, volume `placemark_postgres-data`, mount `/var/lib/postgresql`, port `127.0.0.1:5432`; cold `up -d --wait` reaches healthy; `insert 99` → `docker compose down` (no `-v`) → `up -d --wait` → `select` returns `99`; `ss -ltn` shows `LISTEN 127.0.0.1:5432` and nothing on `0.0.0.0`. `.gitignore` is untouched and my earlier `check-ignore` results still stand. ## Optional nits — do not spin another round for these 1. `docker-compose.yml:15-17` — "the container never starts at all" sits a clause away from "reports the container as Started", which is a mild self-contradiction in a comment whose whole job is precision. It does start; it exits 1 within a second. "never comes up" or "exits 1 within a second" would remove the wobble. Genuinely trivial, and the surrounding sentences make the meaning unambiguous. 2. `docker-compose.yml:2` — the header still says "copy .env.example to change them", which was accurate when the keys were live values. It is now copy *and uncomment*. One word. Both are a single-word edit if you happen to be touching the file anyway; neither justifies a further review cycle. Merge when ready. All Docker state I created is gone — three test projects `down -v`, scratch directories removed, no Postgres port listening. `placemark-placemark-1`, `placemark-mongo` and `placemark_mongo-data` are untouched and exactly as I found them. My worktree is still detached at `fa13a0d` with a clean tree; I read `fcc4fde` via `git show origin/feat/docker-compose:<path>` and never checked it out.

.env.example

					
				@ -0,0 +8,4 @@

				#

				#     cp .env.example .env

				#

				# The values below are deliberately *not* the defaults, so that nothing here can quietly fall out

I asked for deduplication; this is better, and I would not go back to my suggestion.

Deduplicating leaves two artefacts that must agree and depends on someone remembering both. A file that never states the defaults cannot fall out of step with them, because it makes no claim to fall out of step with. That is a stronger property than "currently consistent".

Verified the no-op: cp .env.example .env then docker compose config yields placemark / placemark / placemark on 127.0.0.1:5432 with no warnings — #KEY=value with no space parses as a comment. Uncommenting two keys took effect (choose-your-own, published "55432").

The placeholder values are well chosen: someone_else and choose-your-own are self-evidently not defaults, so nobody uncomments the lot by reflex and then wonders why the README's connection string fails. And lines 3-5 point at the README for the real values, so nothing is lost — only moved, with a signpost.

I asked for deduplication; this is better, and I would not go back to my suggestion. Deduplicating leaves two artefacts that must agree and depends on someone remembering both. A file that never states the defaults cannot fall out of step with them, because it makes no claim to fall out of step with. That is a stronger property than "currently consistent". Verified the no-op: `cp .env.example .env` then `docker compose config` yields `placemark` / `placemark` / `placemark` on `127.0.0.1:5432` with no warnings — `#KEY=value` with no space parses as a comment. Uncommenting two keys took effect (`choose-your-own`, published `"55432"`). The placeholder values are well chosen: `someone_else` and `choose-your-own` are self-evidently not defaults, so nobody uncomments the lot by reflex and then wonders why the README's connection string fails. And lines 3-5 point at the README for the real values, so nothing is lost — only moved, with a signpost.

docker-compose.yml

					
				@ -0,0 +12,4 @@

				    # mounts one level up at /var/lib/postgresql. Do not "correct" it to the pre-18

				    # /var/lib/postgresql/data seen in most tutorials: the entrypoint spots the unused mount and

				    # aborts on the first `up` with exit 1 and an error naming

				    # "/var/lib/postgresql/data (unused mount/volume)". Loud rather than silent, but the container

Verified this whole paragraph rather than reading it, since the previous round turned on exactly this claim — and it is right.

Fresh volume, mount at the legacy path, plain docker compose up -d: output ends Container ... Started and the shell exit code is 0. docker compose ps run immediately after still showed Up Less than a second (health: starting) with the port mapping. ps -a a moment later: Exited (1). The unused mount/volume error appears only in docker compose logs.

So "loud rather than silent, but the cause is not obvious from docker compose up -d" is precisely accurate, and it is a better description than the one I offered in the last round. No change needed.

One trivial wobble, take it or leave it: "the container never starts at all" sits one clause from "reports the container as Started". It does start — it runs the entrypoint and exits 1 within a second. "never comes up" would read cleanly. Not worth a round on its own.

Verified this whole paragraph rather than reading it, since the previous round turned on exactly this claim — and it is right. Fresh volume, mount at the legacy path, plain `docker compose up -d`: output ends ` Container ... Started ` and the shell exit code is **0**. `docker compose ps` run immediately after still showed `Up Less than a second (health: starting)` with the port mapping. `ps -a` a moment later: `Exited (1)`. The `unused mount/volume` error appears only in `docker compose logs`. So "loud rather than silent, but the cause is not obvious from `docker compose up -d`" is precisely accurate, and it is a better description than the one I offered in the last round. No change needed. One trivial wobble, take it or leave it: "the container never starts at all" sits one clause from "reports the container as Started". It does start — it runs the entrypoint and exits 1 within a second. "never comes up" would read cleanly. Not worth a round on its own.

rob merged commit 9061f9ddba into main

2026-08-02 20:16:03 +00:00

rob referenced this pull request from a commit

2026-08-02 20:16:03 +00:00

Add a Docker Compose Postgres for local development (#7)

rob deleted branch feat/docker-compose

2026-08-02 20:16:03 +00:00

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

rob/PlaceMark!7

Rows
Columns

Add a Docker Compose Postgres for local development (Vikunja task #4) #7

Correction to this description's original claim

The volume path

Verification actually performed

Decisions

Traps documented, not fixed

Notes for other tickets

What I verified myself

Judgements you asked for

Non-blocking, in priority order

On the stale placemark project, since you raised it

Finding 1 — the new framing is accurate, not a hedge

Finding 3 — the exit-1 argument for dropping on-failure is correct

Finding 4 — better than what I suggested, and I have changed my mind

Findings 2 and 5 — confirmed in the files, not taken on trust

Re-verification of the unchanged parts

Optional nits — do not spin another round for these

On the stale `placemark` project, since you raised it

Finding 3 — the exit-1 argument for dropping `on-failure` is correct