Skip to content

Outage stream import never persists rows: the imported counter increments without a single DB write #360

Description

@usmanimamu17-create

Problem

stream_import_outages (app/services/outage_stream_import.py) is documented as the streaming import path, but its per-row "insertion" is a placeholder:

for row in chunk:
    try:
        # Basic validation: require site_id
        if not row.get("site_id"):
            failed.append({"index": i, "row": row, "error": "missing site_id"})
            continue
        # In production: validate with Pydantic model and insert via SQLAlchemy
        imported += 1

The comment says it all: nothing validates with Pydantic and nothing inserts via SQLAlchemy. imported is incremented for every row that merely has a site_id key.

Consequences:

  • POST /outages/import (if routed through this helper) reports success for rows that were never stored: the response {"imported": N, "failed_count": 0, ...} is fiction; the outage data does not exist afterward.
  • The streaming claim is also false: the docstring says "Stream bulk outage import using ijson to avoid OOM", but the implementation calls json.loads(raw_body) — the entire body is materialized in memory, so the OOM protection the feature exists for is not provided.
  • The failed_rows index is wrong too: failed.append({"index": i, ...}) uses i, the chunk-start offset, so rows in the second chunk all report index: 100 — operators cannot locate bad rows.

Root cause

The module was scaffolded as a stub (issue #27 reference) and never completed; it is dead code that cannot even be exercised safely.

Why this is architecturally hard

  1. Making it real requires wiring the existing outage ingestion: validate each row with OutageCreate/the bulk-create DTO (app/models/outage_dto.py), dedupe via OutageRepository.create_or_get_existing, and batch-commit with the same all-or-nothing semantics the bulk endpoint uses — plus the correct row index in failure payloads.
  2. True streaming requires ijson incremental parsing of the request body rather than json.loads; the chunked-processing loop currently operates on an already-loaded list, so the "chunking" is cosmetic.
  3. The helper must be tested against the real endpoint contract (/outages/import), including oversized payloads, so the memory behavior is actually verified.

Proposed design

Implement real validation and persistence (reusing OutageCreate, create_or_get_existing, and the repository's dedupe), fix the failure index to the true row position, and either implement ijson-based streaming or remove the streaming claim from the docstring. Add tests asserting imported rows exist in the DB and oversized payloads are handled within the memory bound.

Acceptance criteria

Service

  • Imported counts match rows actually persisted.
  • Failure reports point at the correct row index.

Tests

  • A test imports rows and asserts they are queryable afterward.
  • A test verifies failure indices are row-accurate.
  • A large-payload test bounds memory (or the streaming claim is removed).

Out of scope

CSV import format (tracked separately) and atomicity of the bulk endpoint.

Getting started

pytest tests/test_outage_db_pagination.py -q
make typecheck

Good first files to read: app/services/outage_stream_import.py, app/repositories/outage_repository.py, app/models/outage_dto.py.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardThird CampaignCampaign: Third Campaignarea/dataImported campaign issue labelarea/outagesImported campaign issue labelpriority/highImportant; address in current quarter

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions