A missing Activity, or any integrity error that is not a duplicate, gets the wrong status

Open
  • Is something wrong?
#261 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
72/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
fastapi, postgresql, python
Domain
api, backend, databases

Research direction

Start with the named handlers: CollectionService.create, ShotService.create, SourceService.create, and DeviceService.create, then compare their integrity error handling with the dataset and collection update handlers. Check Activity lookup in dataset and collection create and update, and read tests/api/v1/test_update_integrity.py for the expected behavior. Done means missing Activities return 404 before writes, only unique violations return 409, other integrity errors are re-raised, and the named test expects 404.

Written by the indexing model from the issue text.

Description

What happens

A request whose activity_id names no Activity fails at the database's foreign key, and the API reports it as something else:

  • POST /v1/collections/ with {"name": "c", "activity_id": 999999} answers 409 Collection 'c' already exists in this context.
  • POST /v1/datasets/ with {"name": "d", "activity_id": 999999} answers 500.
  • PATCH /v1/datasets/{id} or PATCH /v1/collections/{id} with {"activity_id": 999999} answers 500.

A derived_from entry naming a dataset that does not exist already answers 404 (Dataset 999999 not found). A missing Activity should answer the same way, before anything is written.

Why

CollectionService.create turns every IntegrityError into a ConflictError, so a foreign-key violation reads as a duplicate name. Since #236, the dataset handler and the collection update map only unique violations to 409 and re-raise anything else, so the same request fails there with 500.

Three other create handlers also treat every IntegrityError as a duplicate: ShotService.create, SourceService.create and DeviceService.create. Today only a duplicate reaches them, because they look the device up first, but any constraint added to those tables later would be reported as "already exists".

What to change

  • On dataset and collection create and update, look up the Activity named by activity_id and raise ResourceNotFoundError (404) if it does not exist.
  • In the collection, shot, source and device create handlers, map only psycopg.errors.UniqueViolation to 409, and roll back and re-raise anything else, as the dataset and collection update handlers do.
  • test_update_missing_activity_preserves_integrity_error in tests/api/v1/test_update_integrity.py expects the 500. Change it to expect 404.
Dominant language
Python
Stars
0
Forks
3
Avg merge
23h
Merged PRs (30d)
70

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from ukaea/fds

All issues in ukaea/fds

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.