vsima opened a new pull request, #73411:
URL: https://github.com/apache/airflow/pull/73411
Document the 409 on `POST /dags/{dag_id}/favorite`, and make its body match
its sibling.
Two related problems on the same endpoint pair.
**The published spec contradicts the project's own test.** `favorite_dag`
declares `create_openapi_http_exception_doc([404])` while `unfavorite_dag`
declares `[404, 409]`. Favoriting an already-favorited Dag does return 409 —
`test_favoriting_already_favorited_dag_returns_409` has asserted exactly that
since the endpoint was added in #51264. Because the response was never
declared, it is
absent from `v2-rest-api-generated.yaml`, and so from every client generated
from it: a generated client sees an undeclared status and surfaces it as an
unexpected error rather than "already a favorite".
**The two endpoints answer the same conflict differently.** `unfavorite_dag`
checks for the row and raises a 409 with `Dag is not marked as favorite`.
`favorite_dag` has no check, so the duplicate insert violates the unique
constraint and is caught by the generic `_UniqueConstraintErrorHandler`. The
body a user gets for favoriting twice is:
```json
{
"message": "Serious error when handling your request. Check logs for more
details - you will find it in api server when you look for ID 6Q4Msafe",
"orig_error": "hidden",
"reason": "Unique constraint violation",
"statement": "hidden"
}
```
Favoriting something that is already a favorite is a stale toggle (a second
tab, a cached list, an API client retrying), not a serious error, and it
does not warrant sending an operator into the API
server logs after a correlation ID. The same user error produces a clean,
translatable message in one direction and this in the other.
This adds the existence check to `favorite_dag`, mirroring `unfavorite_dag`
twenty lines below, and declares the 409.
**Behaviour change, stated plainly:** the status code does not change. The
409's `detail` changes from the handler's object above (which also carries
the
SQL statement and dialect error when `[api] expose_stacktrace` is on) to the
string `Dag is already marked as favorite`, the same shape `unfavorite_dag`
returns. A repeat favorite also stops writing an ERROR-level line to the API
server log. Anything matching on the old payload would be affected; the only
in-tree references to `Unique constraint violation` are the handler's own
tests
in `test_exceptions.py`, which do not touch this endpoint, and the UI does
not
branch on either message.
**On the race:** the check is not atomic, so two concurrent favorite calls
can
still reach the unique constraint and return the handler's 409. That is the
same window `unfavorite_dag` has, and the status is unchanged either way, so
this keeps the existing pattern rather than introducing a different one here.
Happy to take the `ON CONFLICT DO NOTHING` route instead if reviewers prefer.
**Tests:** `test_favoriting_already_favorited_dag_returns_409` now asserts
the
body as well as the status. I also added the matching assertion to
`test_unfavoriting_dag_that_is_not_favorite_returns_409` — strictly it tests
existing behaviour, but it is the other half of the symmetry this PR is
claiming, so it seemed worth pinning. The new probe gets
`test_favorite_dag_existence_check_is_bounded`, the twin of the existing
unfavorite test, so both existence checks are held to `LIMIT 1`.
Verified locally: `pytest
tests/unit/api_fastapi/core_api/routes/public/test_dags.py -k favorite`
gives 17 passed. Reverting the route change alone makes
`test_favoriting_already_favorited_dag_returns_409` fail on the body, which
is
where the payload above came from, so the assertion is pinned to the fix
rather
than passing incidentally.
The spec and the UI client (`types.gen.ts`, `services.gen.ts`) are the output
of `run_generate_openapi_spec.py` and `pnpm codegen`; nothing else in the
generated files moved.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (Claude Code)
Generated-by: Claude Code following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
