#129 - importer: top-level subcommands (import / validate-config / acceptance / fetch) — a misread flag must not launch a multi-hour full import - art/ismap

art commented

2026-08-15 14:03:07 +00:00

Owner

Scope: the importer/importer CLI — its flag surface, docs/importer-cli.md, and the incident that makes the current shape dangerous.

What happened (issue #113, 2026-08-15)

A worker wanted a cheap config sanity check after editing conf/fimap.yaml and ran:

./importer/importer -validate conf/fimap.yaml

-validate is not a validation mode — it is a full-import modifier: "after each table is written, check ST_IsValid and log any invalid geometries". The command parsed as "run the complete Finland import, with geometry checks": every staging table, every layer, every zoom, ~3h12m of postgres at ~98% CPU on the cover cascade before the driver killed it. The change being checked touched three layers.

The help output is the trap: importer -h prints 17 flat flags in one list. There is no "this is a mode, this is a modifier" structure, and the -validate description sat below the 30-line truncation the worker used. -h also does not say that importer <config> alone means "full import of the whole country".

Why the current shape is a footgun

Bool flags silently upgrade a full import. -validate, -ensure, -refetch change what an already-destructive default (importer conf/fimap.yaml = full import) does, so a flag added to inspect can instead modify. The failure mode is invisible until hours are gone — nothing prints a confirmation of what mode is about to run.
"Do X then exit" modes are flags, not verbs. -acceptance-only, -geotorget-fetch, -geotorget-list, -lastkajen-fetch, -lastkajen-list, -trafiklab-fetch are six different programs hiding behind -flag spelling; a typo (-validate for a config check) lands in the one remaining verb, "import everything".
The destructive default is unlabelled. importer <config> with no flags is the multi-hour full import; the help never says so in one line.
#98's proposed -check-queries guard would be another mode bolted onto the same list.

Proposed shape (top-level subcommands, flags become modifiers)

importer help [command]
importer import   <config> [--layer a,b,c] [--regeneralize 5-11] [--validate] [--ensure] [-jobs -batch-rows]
importer validate-config <config>     # config sanity only — no DB writes (the check #113 meant to run)
importer acceptance <config>          # current -acceptance-only
importer check-queries <config>       # #98's PREPARE guard, when it lands
importer fetch <config> [geotorget|lastkajen|trafiklab|all]   # current *-fetch modes
importer list  <config> [geotorget|lastkajen]                 # current *-list modes

Key properties to preserve:

importer with no verb prints help (the current -h), never starts work.
A subcommand that writes the DB (import) should state its blast radius before doing it, at least in help and ideally in a startup line ("full import of — N layers, M zooms" vs "layers a,b,c only").
A config-only check must exist as a real cheap mode so nobody reaches for -validate again (that gap is what caused this).
Backward compatibility for the existing flags is a decision, not a requirement — the Makefile is the only caller worth keeping working (make import*), and it is in this repo.

Decisions needed

Subcommand set and names (above is a proposal; validate-config may be config-check).
How much compatibility to keep for the flat flags (deprecate-with-warning vs immediate break; the Makefile targets are the whole consumer list).
Whether the destructive-mode startup line is worth an acceptance-style test of its own.
Doc work: docs/importer-cli.md gains a mode table; the acceptance-tests section stays.

Notes

The incident itself is fixed at the behaviour level by #113's worker using -layer places,railways,stations (the per-layer path, minutes not hours); this issue is the CLI-shape fix so the same misread cannot happen to anyone.
Related backlog: #98 (the -check-queries guard) slots naturally into the subcommand shape. #114 (staging where:) is unaffected but its docs change touches the same file.

Scope: the `importer/importer` CLI — its flag surface, `docs/importer-cli.md`, and the incident that makes the current shape dangerous. ## What happened (issue #113, 2026-08-15) A worker wanted a cheap config sanity check after editing `conf/fimap.yaml` and ran: ``` ./importer/importer -validate conf/fimap.yaml ``` `-validate` is not a validation mode — it is a **full-import modifier**: "after each table is written, check ST_IsValid and log any invalid geometries". The command parsed as "run the complete Finland import, with geometry checks": every staging table, every layer, every zoom, ~3h12m of postgres at ~98% CPU on the cover cascade before the driver killed it. The change being checked touched three layers. The help output is the trap: `importer -h` prints 17 flat flags in one list. There is no "this is a mode, this is a modifier" structure, and the `-validate` description sat below the 30-line truncation the worker used. `-h` also does not say that `importer <config>` alone means "full import of the whole country". ## Why the current shape is a footgun - **Bool flags silently upgrade a full import.** `-validate`, `-ensure`, `-refetch` change what an already-destructive default (`importer conf/fimap.yaml` = full import) does, so a flag added to *inspect* can instead *modify*. The failure mode is invisible until hours are gone — nothing prints a confirmation of what mode is about to run. - **"Do X then exit" modes are flags, not verbs.** `-acceptance-only`, `-geotorget-fetch`, `-geotorget-list`, `-lastkajen-fetch`, `-lastkajen-list`, `-trafiklab-fetch` are six different programs hiding behind `-flag` spelling; a typo (`-validate` for a config check) lands in the one remaining verb, "import everything". - **The destructive default is unlabelled.** `importer <config>` with no flags is the multi-hour full import; the help never says so in one line. - **#98's proposed `-check-queries` guard** would be another mode bolted onto the same list. ## Proposed shape (top-level subcommands, flags become modifiers) ``` importer help [command] importer import <config> [--layer a,b,c] [--regeneralize 5-11] [--validate] [--ensure] [-jobs -batch-rows] importer validate-config <config> # config sanity only — no DB writes (the check #113 meant to run) importer acceptance <config> # current -acceptance-only importer check-queries <config> # #98's PREPARE guard, when it lands importer fetch <config> [geotorget|lastkajen|trafiklab|all] # current *-fetch modes importer list <config> [geotorget|lastkajen] # current *-list modes ``` Key properties to preserve: - `importer` with no verb prints `help` (the current `-h`), never starts work. - A subcommand that writes the DB (`import`) should state its blast radius before doing it, at least in help and ideally in a startup line ("full import of <cc> — N layers, M zooms" vs "layers a,b,c only"). - A config-only check must exist as a real cheap mode so nobody reaches for `-validate` again (that gap is what caused this). - Backward compatibility for the existing flags is a decision, not a requirement — the Makefile is the only caller worth keeping working (`make import*`), and it is in this repo. ## Decisions needed 1. Subcommand set and names (above is a proposal; `validate-config` may be `config-check`). 2. How much compatibility to keep for the flat flags (deprecate-with-warning vs immediate break; the Makefile targets are the whole consumer list). 3. Whether the destructive-mode startup line is worth an acceptance-style test of its own. 4. Doc work: `docs/importer-cli.md` gains a mode table; the acceptance-tests section stays. ## Notes - The incident itself is fixed at the behaviour level by #113's worker using `-layer places,railways,stations` (the per-layer path, minutes not hours); this issue is the CLI-shape fix so the same misread cannot happen to anyone. - Related backlog: #98 (the `-check-queries` guard) slots naturally into the subcommand shape. #114 (staging `where:`) is unaffected but its docs change touches the same file.

art commented

2026-08-15 14:12:07 +00:00

Author

Owner

Some decisions:

config-check sounds better.
immediate break, you don't disarm a footgun by keeping compatibility around.
I wouldn't call it destructive, a full import should always be safe, it's just slow.
Yes, of course, always update the docs.

Some decisions: 1. `config-check` sounds better. 2. immediate break, you don't disarm a footgun by keeping compatibility around. 3. I wouldn't call it destructive, a full import should always be safe, it's just slow. 4. Yes, of course, always update the docs.

art referenced this issue

2026-08-15 14:13:27 +00:00

Catch a layer query reading a column the source fetch does not request #98