Skip to content

docs(api): add API conventions, migrations, and README#231

Open
jacomago wants to merge 1 commit into
ChannelFinder:masterfrom
jacomago:api-conventions
Open

docs(api): add API conventions, migrations, and README#231
jacomago wants to merge 1 commit into
ChannelFinder:masterfrom
jacomago:api-conventions

Conversation

@jacomago

@jacomago jacomago commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Document API conventions including RSQL query syntax, bulk operations, jobs/batch definitions, and migration notes under dev-docs/api/.

Document API conventions including RSQL query syntax, bulk operations,
jobs/batch definitions, and migration notes under dev-docs/api/.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Jul 6, 2026

Copy link
Copy Markdown

@anderslindho

Copy link
Copy Markdown
Contributor

Overall the v1 direction here is solid: plural nouns, correct method semantics, RFC 9457 errors, split request/response schemas, jobs for async work, RSQL replacing the old boolean-DSL magic chars. All good moves, agree with merging that as the baseline.

Where I think this needs another pass is the batch/bulk section, specifically re how it's meant to actually serve RecCeiver.

batch vs reconcile

:batch's mixed post/put/patch/delete shape still needs the producer to know what already exists in CF (to bucket into post vs put) and what to delete (no query-scoped delete, no server-side reconcile, per the doc's own wording). That's the same read-before-write pattern we're trying to get RecCeiver away from. The doc even says it outright: "a producer that does not track what it holds builds the operations by diffing its desired set against a lean GET /channels...". If RecCeiver still has to GET-then-diff to build the batch, that's the same read, just done before submitting instead of after.

Proposal: add an actual reconcile op. Producer submits "this is my desired/observed set for scope X, generation N," CF upserts and tombstones anything in-scope not mentioned. That's the real one-way primitive. Once that exists I think the mixed-method :batch shape can go away entirely, reconcile covers what it was trying to do for RecCeiver, and I don't think anyone else needs mixed-method batch (open Q below).

Reconcile doesn't get to skip jobs/per-item results just because it's declarative. Still large-N (thousands of channels per IOC per our own README numbers), still partially-failable, and a failed item must not get tombstoned (it didn't disappear, it just failed validation), so reconcile needs per-item results plus sync-under-threshold/async-over-threshold, same machinery batch has today, just wrapped around a "desired state" body instead of an op-list.

generic (non-reconcile) bulk, leaving this open

Once reconcile absorbs RecCeiver: is there still a real need for generic bulk create/update/delete (admin tooling, migration scripts, whatever)? If yes, and it's small-N/interactive (our own actor table already has admin/curator as "occasional, deliberate"), I don't think that needs jobs or per-item results at all. Validate-then-write, atomic, single response and status. Simpler for callers than today's 200-with-embedded-422s. If there's actually a consumer doing large/failure-prone homogeneous bulk writes outside RecCeiver, that path needs per-item handling too, same as reconcile, just without tombstoning. Need to know if that consumer exists before deciding.

200 vs 207 vs job, the doc contradicts itself here

Hard rules say "never 200 with an error body." Async section says "never... a 207 Multi-Status." Sync :batch's own example is a 200 with embedded per-item 422s. All three can't be true at once. If the intent is ":batch is a command, 200 = command ran, per-item statuses are data not errors," fine, but that needs to be written as an explicit carve-out to the hard rule, not left implicit.

"mirrors ES _bulk", ground this in domain not backend

Non-atomicity and the 200-status choice for batch are both justified by "follows Elasticsearch _bulk," but the doc is explicit elsewhere (opaque cursor) that engine behavior shouldn't leak into the contract. Same conclusion probably still holds, just ground it properly: large batches need forward progress, one bad item shouldn't sink 5k good ones, and no backend gives free cross-doc atomicity at this scale anyway. That reasoning would survive a hypothetical backend swap. The ES justification wouldn't.

native concepts needed for reconcile

For tombstoning to be safe I think we need, minimum:

  • an opaque source_id/scope key, producer-supplied, CF doesn't interpret it (same idea as the opaque pagination cursor already in here)
  • a generation/sequence guard per scope, so a delayed or reordered reconcile submission can't tombstone state a newer one already established
  • first-class lifecycle fields on the channel (state, last_seen_at, tombstoned_at) instead of inferring lifecycle from arbitrary metadata, makes purge policy simpler to query, lets GET /channels default-exclude tombstoned channels, and gives an audit trail without extra work

Don't think we need IOC / RecCeiver / session to become their own CF resources though, scope just needs to be whatever unit appears/disappears together. RecCeiver owns getting that scoping right. CF just needs the opaque key plus generation guard.

one more thing this forces us to decide now, not later: can one channel have multiple hosts?

Separately from all of the above, PVs can be hosted by more than one IOC at once. Today CF pretends this can't happen, so when more than one recceiver/IOC reports the same name, whichever write lands last just silently clobbers the previous metadata. No visibility that it happened.

This lands directly on the source_id-scoped reconcile design above, not just future work. As sketched, if source A and source B both claim channel X, B's reconcile submission would silently overwrite A's metadata for X, and A's next reconcile would silently overwrite it back. Same bug we're trying to kill, just now inside the new primitive instead of the old one. So reconcile can't be specced without deciding: is an upsert a per-source claim on a channel, or a global clobber?

I think existence and metadata split cleanly here. Existence can aggregate now: channel exists if ≥1 source claims it, tombstone only when the last claim drops, that's just reference counting, solvable today. Metadata conflict is harder: two sources reporting different properties/tags for the same name doesn't aggregate cleanly, and I don't think we need to solve that now (whether we warn, reject, or just surface both isn't decided). But the identity question, keyed by name alone vs. some (name, ioc_id)-style composite, exact keying TBD, needs to at least be flagged as open now, because retrofitting single-owner -> multi-claim later is a lot more expensive than leaving room for it in the reconcile primitive from the start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants