Bounded list limits and NDJSON streaming exports - #14
Open
NathanCummings wants to merge 8 commits into
Open
NathanCummings wants to merge 8 commits into
NathanCummings wants to merge 8 commits into
Conversation
Pull the access-policy logic in DatasetService, ShotService, and CollectionService into a private `_resolve_access(obj, user) -> str | None` method that returns None on success or a denial reason on failure. `check_read_access` now raises ForbiddenError(reason) on a non-None result; the new `is_accessible(obj, user) -> bool` returns `_resolve_access(...) is None`. Both surface the same canonical logic without duplication and without exception-driven control flow in the predicate's hot path — required by ADR-0020's streaming export, which calls `is_accessible` once per yielded row and cannot afford an exception after response headers have been sent. Also collapses each list endpoint's `_filter_accessible_*` helper into a one-line list comprehension over the predicate. Adds tests/services/test_access_predicates.py covering predicate purity (returns bool, never raises) across all access tiers and a broken-inheritance fallback case.
Add app/api/streaming.py exposing serialize_ndjson() and ndjson_response()
for the bulk-export endpoints (ADR-0020). One JSON object per line,
trailing newline, application/x-ndjson media type. Pydantic's
model_dump_json(exclude_none=True) drives the per-row serialisation.
Lives in app/api/ rather than app/core/ because it imports
fastapi.responses.StreamingResponse — keeps the web-framework dependency
out of app/core/ where everything else is web-framework-agnostic.
Add stream() / stream_by_device_name() to DatasetService, ShotService,
and CollectionService. Each:
- Uses SQLAlchemy 2.0's stmt.execution_options(yield_per=...) to keep
the API process's memory bounded regardless of corpus size.
- Eager-loads the access-level inheritance chain via selectinload()
so the per-row read-model conversion does not trigger N+1 queries
inside the stream. Dataset.distributions, Dataset.shot.device, and
Collection.shot.device are batched per chunk.
- Calls is_accessible() for per-row filtering — pure predicate, never
raises, so a per-row failure cannot abort an in-flight HTTP response
after headers have been sent.
Add CollectionService.to_flat_read_model() — produces an export-shaped
CollectionRead with effective_access_level resolved but datasets and
child_collections set to None. Lives on the service so the export
router stays free of auth/model imports; mirrors how DatasetService
and ShotService already drive their export read-model conversion.
Adds tests/api/test_streaming.py covering NDJSON shape, exclude_none
behaviour, lazy iteration, and the content-type contract.
No HTTP surface change in this commit — the service stream() methods
are unused until the /export endpoints land in the next commit.
Wire the streaming machinery into HTTP endpoints, completing ADR-0020.
Each handler is a thin three-call chain:
service.stream(...) → service.to_*read_model(...) → ndjson_response(rows).
- GET /api/v1/datasets/export — optional device_name and shot_id query
filters mirror the path-based list endpoints.
- GET /api/v1/devices/{device_name}/shots/export
- GET /api/v1/collections/export — emits a flat CollectionRead via
CollectionService.to_flat_read_model() (datasets and child_collections
set to None) so per-row payloads stay bounded; clients pull membership
separately by Collection ID.
Auth posture mirrors the corresponding list endpoint: open to anonymous
callers, with per-row access filtering silently dropping records the
caller cannot see. Empty result sets return 200 OK with an empty body.
Invalid tokens still 401 during dependency resolution before the
handler runs; once StreamingResponse headers are sent, the per-row
predicate stays raise-free so a per-row failure cannot abort the
in-flight response.
Adds tests/api/v1/test_export_streaming.py covering NDJSON shape,
content-type, anonymous + admin filtering, device/shot scope filters,
empty-corpus 200 OK, and a broken-inheritance fallback case where an
orphaned dataset is silently dropped from an anonymous caller's stream
without raising.
ADR-0020 status: Proposed → Accepted.
This was referenced Sep 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Carried over from GitLab MR !2, which was in conflict with
mainthere. Eight commits: request limits are clamped to a configured bound, read-access policy is centralised in_resolve_access, and datasets, shots and collections gain/exportendpoints that stream NDJSON.Known from the GitLab review: the
/exportgenerators emitapplication/jsonland ignoreresponse_modeloptions such asexclude_none, since FastAPI does not apply them to streamed responses. That is unresolved here and is the main thing to settle before merging.The branch predates the recent changes to
mainand will need rebasing; the conflicts are the ones GitLab reported.🤖 Generated with Claude Code
https://claude.ai/code/session_013E6ygEUq3gzUrTzPJxcwtH