Decouple config validation from I/O; check is the sole completeness gate
What changed
bench check accepted inbound endpoints missing the field that makes them
addressable: an HTTP endpoint with no route, a gRPC one with no service_name,
a NATS one with no subject. Each parses as a well-formed message but describes
nothing that could serve a request, which is exactly the mistake check exists
to catch.
Fixing that surfaced a deeper problem. Validation was baked into every read and
write, ran a full validate(), and rejected any incomplete spec. The issue is that
bench set builds a spec one field at a time, and each call is a read-modify-write,
so a strict completeness rule made the intermediate file unreadable for the next set.
This was exhibited by the need to do bench set assemblies.foo-module.listeners.0 '{"port":8080,"protocol":"PROTOCOL_HTTP"}'
in the test.sh since both fields were required on the listener object.
Scope
The MR now separates the two questions that coupling conflated:
- Can the config be read or written at all? Read only parses; write only serializes. An incomplete spec round-trips freely.
- Is the spec a complete, runnable service? That's
bench check's job, run explicitly (and byscaffold/build), never on config I/O.
Validation severity
Findings carry a severity, and there are two:
| Level | Fails check |
Fails check --strict |
|---|---|---|
error |
yes | yes |
warning |
no | yes |
An error is a missing required field or an impossible value (a port out of
range, a duplicate handler identity). A warning is an advisory that doesn't
block a build. The only warning today is the shared-handler smell from #46: one
handler func bound to several distinct routes. --strict (which CI runs)
promotes warnings to failures.
An earlier revision of this MR carried a third incomplete level to let set
write unfinished specs. Once validation came off the I/O path that level had no
job left to do, so it's gone.
The listener composite, fixed
Before this change, building a listener needed a single composite value because both fields were required at write time:
$ bench set assemblies.foo-module.listeners.0 '{"port":8080,"protocol":"PROTOCOL_HTTP"}'Now each field writes on its own, like any other object, and check enforces
both are present:
$ bench set assemblies.foo-module.listeners.0.port 8080 # writes; protocol still missing
$ bench check
error: ...listeners[0].protocol: protocol is required (PROTOCOL_HTTP or PROTOCOL_GRPC) # exit 1
$ bench set assemblies.foo-module.listeners.0.protocol PROTOCOL_HTTP
$ bench check # exit 0tests/test.sh splits the listener accordingly and wires the handlers that
check now requires on every endpoint.
API notes for reviewers
Click to expand
validate()returnsValidationErrorsdirectly (empty means valid) instead of aResult.Errused to mean "blocks I/O"; nothing blocks I/O on completeness anymore.- Removed
Severity::Incomplete,ValidationErrors::blocks_io,blocking(), andpush_incomplete. Addedhas_errors(). read_*/write_*helpers no longer validate; their doc comments say so.setno longer validates.checkandscaffoldcallvalidate()explicitly;scaffoldfails on errors but tolerates warnings.
Closes #42 (closed)