Skip to content

Inventory upload status flips to "completed" before raw file cleanup finishes #551

Description

@lindsaywiard

Found this while investigating a CI failure on #544:

tests/resources/inventories/tree/upload/test_router.py::TestUploadFullFlow::test_upload_completes[csv] intermittently fails:

AssertionError: assert not True
 +  where True = exists('gs://.../inventories/9cbdf3d21d5f4cf3bf64b7548e23a34e/upload.csv')

The test polls until the inventory's status is completed, then asserts the raw uploaded file no longer exists. A rerun of the same test passed cleanly, so this isn't deterministic.

Hypothesis

In handle_inventory (services/uploader/uploader/handlers/inventory.py), the Firestore status: "completed" write happens inside the try block, and the raw-file cleanup (delete_file) happens afterward, in finally:

Details

try:
    ...
    update_document(INVENTORIES_COLLECTION, resource_id, {"status": "completed", ...})
finally:
    try:
        delete_file(f"gs://{bucket}/{object_name}")
    except Exception:
        pass

Since the Firestore write and the GCS delete are separate, unrelated network calls with no atomicity between them, a poller could observe "completed" in the window before "delete_file" has actually run — which would explain the failure.

Fix

We could either:

  • Delete the raw file before writing status: "completed", so "completed" only becomes visible once cleanup has actually happened.
  • Or just change the test — if nothing actually depends on the raw file being gone the instant status flips to completed, retry the existence check a few times instead of asserting immediately.

Metadata

Metadata

Assignees

No one assigned

    Labels

    🐛 cold fixThere's a bug, but it's not critical to fix right away💻 API ServiceAPI service

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions