This is an automated email from the ASF dual-hosted git repository.
pierrejeambrun pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 8d4ab8a94db Keep OpenAPI error responses in sync with the statuses
routes raise (#71647)
8d4ab8a94db is described below
commit 8d4ab8a94db291a359b686ac545211f4c440cb3e
Author: Pushkal Gupta <[email protected]>
AuthorDate: Wed Sep 30 22:05:13 2026 +0530
Keep OpenAPI error responses in sync with the statuses routes raise (#71647)
* Keep OpenAPI error responses in sync with the statuses routes raise
`create_openapi_http_exception_doc(...)` feeds the `responses=` block that
the
generated spec — and every client built from it — uses to model error
responses, but nothing ties that list to the statuses a handler actually
raises. The two drift apart silently, and the same drift has had to be found
and patched by hand four times (#67570, #67571, #70992, #71011).
A static check keeps them together, so the next divergence fails in CI
instead
of shipping a spec that omits a response the API really returns.
* Recognize Cadwyn routers in the OpenAPI error-response check
ROUTER_CLASSES omitted VersionedAPIRouter, so router-level `responses=` was
invisible on every Cadwyn-based execution API route, and three statuses those
routers already declare were reported as undeclared.
prek executes hook scripts directly, so the script needs its executable bit
to run at all.
---
airflow-core/.pre-commit-config.yaml | 6 +
.../api_fastapi/core_api/openapi/_private_ui.yaml | 18 ++
.../core_api/openapi/v2-rest-api-generated.yaml | 18 ++
.../core_api/routes/public/connections.py | 6 +-
.../api_fastapi/core_api/routes/public/log.py | 2 +-
.../core_api/routes/public/variables.py | 2 +-
.../api_fastapi/core_api/routes/ui/assets.py | 2 +
.../core_api/routes/ui/partitioned_dag_runs.py | 1 +
.../api_fastapi/core_api/routes/ui/teams.py | 2 +
.../execution_api/routes/asset_events.py | 8 +-
.../api_fastapi/execution_api/routes/hitl.py | 8 +-
.../execution_api/routes/task_instances.py | 4 +
.../api_fastapi/execution_api/routes/xcoms.py | 12 +
.../ui/openapi-gen/requests/services.gen.ts | 6 +
.../airflow/ui/openapi-gen/requests/types.gen.ts | 24 ++
.../ci/prek/check_openapi_exception_doc_in_sync.py | 239 +++++++++++++++
.../test_check_openapi_exception_doc_in_sync.py | 335 +++++++++++++++++++++
17 files changed, 688 insertions(+), 5 deletions(-)
diff --git a/airflow-core/.pre-commit-config.yaml
b/airflow-core/.pre-commit-config.yaml
index d7b96c960d7..11e70570598 100644
--- a/airflow-core/.pre-commit-config.yaml
+++ b/airflow-core/.pre-commit-config.yaml
@@ -165,6 +165,12 @@ repos:
^src/airflow/utils/serve_logs/.*\.py$|
^tests/unit/api_fastapi/.*\.py$|
^tests/unit/utils/test_serve_logs\.py$
+ - id: check-openapi-exception-doc-in-sync
+ name: Check API routes declare the HTTP statuses they raise
+ entry: ../scripts/ci/prek/check_openapi_exception_doc_in_sync.py
+ language: python
+ pass_filenames: true
+ files: ^src/airflow/api_fastapi/.*/routes/.*\.py$
- id: create-missing-init-py-files-tests
name: Create missing init.py files in tests
entry: ../scripts/ci/prek/check_init_in_tests.py
diff --git
a/airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml
b/airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml
index 3c96ebf967f..9bf598b3b10 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml
+++ b/airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml
@@ -304,6 +304,12 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/NextRunAssetsResponse'
+ '404':
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/HTTPExceptionResponse'
+ description: Not Found
'422':
description: Validation Error
content:
@@ -445,6 +451,12 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/PartitionedDagRunDetailResponse'
+ '404':
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/HTTPExceptionResponse'
+ description: Not Found
'422':
description: Validation Error
content:
@@ -2336,6 +2348,12 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/TeamCollectionResponse'
+ '403':
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/HTTPExceptionResponse'
+ description: Forbidden
'422':
description: Validation Error
content:
diff --git
a/airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml
b/airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml
index 05928a631af..c336ff3afa9 100644
---
a/airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml
+++
b/airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml
@@ -1989,6 +1989,12 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/HTTPExceptionResponse'
+ '400':
+ description: Bad Request
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/HTTPExceptionResponse'
'422':
description: Validation Error
content:
@@ -10197,6 +10203,12 @@ paths:
schema:
$ref: '#/components/schemas/HTTPExceptionResponse'
description: Forbidden
+ '404':
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/HTTPExceptionResponse'
+ description: Not Found
'409':
content:
application/json:
@@ -10345,6 +10357,12 @@ paths:
schema:
$ref: '#/components/schemas/HTTPExceptionResponse'
description: Forbidden
+ '400':
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/HTTPExceptionResponse'
+ description: Bad Request
'404':
content:
application/json:
diff --git
a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/connections.py
b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/connections.py
index 082cdd26173..d803b85b191 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/connections.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/connections.py
@@ -328,7 +328,11 @@ def patch_connection(
return connection
-@connections_router.post("/test",
dependencies=[Depends(requires_access_connection(method="POST"))])
+@connections_router.post(
+ "/test",
+ responses=create_openapi_http_exception_doc([status.HTTP_400_BAD_REQUEST]),
+ dependencies=[Depends(requires_access_connection(method="POST"))],
+)
def test_connection(
test_body: ConnectionBody,
user: GetUserDep,
diff --git a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/log.py
b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/log.py
index 6b41aa3f5ac..da8a47dbe50 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/log.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/log.py
@@ -79,7 +79,7 @@ def _buffered_ndjson_stream(
@task_instances_log_router.get(
"/{task_id}/logs/{try_number}",
responses={
- **create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+ **create_openapi_http_exception_doc([status.HTTP_400_BAD_REQUEST,
status.HTTP_404_NOT_FOUND]),
status.HTTP_200_OK: {
"description": "Successful Response",
"content": ndjson_example_response_for_get_log,
diff --git
a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/variables.py
b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/variables.py
index ab753437614..99cd700e3d8 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/variables.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/variables.py
@@ -165,7 +165,7 @@ def patch_variable(
@variables_router.post(
"",
status_code=status.HTTP_201_CREATED,
- responses=create_openapi_http_exception_doc([status.HTTP_409_CONFLICT]),
+ responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND,
status.HTTP_409_CONFLICT]),
dependencies=[Depends(action_logging()),
Depends(requires_access_variable("POST"))],
)
def post_variable(
diff --git a/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/assets.py
b/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/assets.py
index 9fb73471300..a7d36896daa 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/assets.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/assets.py
@@ -47,6 +47,7 @@ from airflow.api_fastapi.core_api.datamodels.ui.assets import
(
NextRunAssetEventResponse,
NextRunAssetsResponse,
)
+from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
from airflow.api_fastapi.core_api.routes.public.assets import OnlyActiveFilter
from airflow.api_fastapi.core_api.security import (
ReadableAssetsFilterDep,
@@ -146,6 +147,7 @@ def get_assets(
@assets_router.get(
"/next_run_assets/{dag_id}",
+ responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
dependencies=[Depends(requires_access_asset(method="GET")),
Depends(requires_access_dag(method="GET"))],
)
def next_run_assets(
diff --git
a/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/partitioned_dag_runs.py
b/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/partitioned_dag_runs.py
index d2141ae5b0d..77ac9454b6a 100644
---
a/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/partitioned_dag_runs.py
+++
b/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/partitioned_dag_runs.py
@@ -363,6 +363,7 @@ def get_partitioned_dag_runs(
@partitioned_dag_runs_router.get(
"/pending_partitioned_dag_run/{dag_id}",
+ responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
dependencies=[Depends(requires_access_asset(method="GET")),
Depends(requires_access_dag(method="GET"))],
)
def get_pending_partitioned_dag_run(
diff --git a/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/teams.py
b/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/teams.py
index 32cc975e96a..2840954c1ee 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/teams.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/teams.py
@@ -29,6 +29,7 @@ from airflow.api_fastapi.common.parameters import (
)
from airflow.api_fastapi.common.router import AirflowRouter
from airflow.api_fastapi.core_api.datamodels.ui.teams import
TeamCollectionResponse, TeamResponse
+from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
from airflow.api_fastapi.core_api.security import (
ReadableTeamsFilterDep,
requires_authenticated,
@@ -41,6 +42,7 @@ teams_router = AirflowRouter(tags=["Teams"], prefix="/teams")
@teams_router.get(
path="",
+ responses=create_openapi_http_exception_doc([status.HTTP_403_FORBIDDEN]),
dependencies=[Depends(requires_authenticated())],
)
def list_teams(
diff --git
a/airflow-core/src/airflow/api_fastapi/execution_api/routes/asset_events.py
b/airflow-core/src/airflow/api_fastapi/execution_api/routes/asset_events.py
index 4fd73b30672..476344a00ee 100644
--- a/airflow-core/src/airflow/api_fastapi/execution_api/routes/asset_events.py
+++ b/airflow-core/src/airflow/api_fastapi/execution_api/routes/asset_events.py
@@ -29,6 +29,7 @@ from airflow.api_fastapi.common.parameters import (
QueryAssetEventPartitionKeyRegex,
)
from airflow.api_fastapi.common.types import UtcDateTime
+from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
from airflow.api_fastapi.execution_api.datamodels.asset import AssetResponse
from airflow.api_fastapi.execution_api.datamodels.asset_event import (
AssetEventResponse,
@@ -106,7 +107,12 @@ def _parse_extra_params(extra: list[str] | None) ->
dict[str, str]:
return result
[email protected]("/by-asset")
[email protected](
+ "/by-asset",
+ responses=create_openapi_http_exception_doc(
+ [(status.HTTP_400_BAD_REQUEST, "Neither name nor uri was supplied")]
+ ),
+)
def get_asset_event_by_asset_name_uri(
name: Annotated[str | None, Query(description="The name of the Asset")],
uri: Annotated[str | None, Query(description="The URI of the Asset")],
diff --git a/airflow-core/src/airflow/api_fastapi/execution_api/routes/hitl.py
b/airflow-core/src/airflow/api_fastapi/execution_api/routes/hitl.py
index 5d34cc6921f..eb70fbd60ad 100644
--- a/airflow-core/src/airflow/api_fastapi/execution_api/routes/hitl.py
+++ b/airflow-core/src/airflow/api_fastapi/execution_api/routes/hitl.py
@@ -25,6 +25,7 @@ from sqlalchemy import select
from airflow._shared.timezones import timezone
from airflow.api_fastapi.common.db.common import SessionDep
+from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
from airflow.api_fastapi.execution_api.datamodels.hitl import (
HITLDetailRequest,
HITLDetailResponse,
@@ -119,7 +120,12 @@ def _check_hitl_detail_exists(hitl_detail_model:
HITLDetail | None) -> HITLDetai
return hitl_detail_model
[email protected]("/{task_instance_id}")
[email protected](
+ "/{task_instance_id}",
+ responses=create_openapi_http_exception_doc(
+ [(status.HTTP_409_CONFLICT, "A response has already been received for
this HITLDetail")]
+ ),
+)
def update_hitl_detail(
task_instance_id: UUID,
payload: UpdateHITLDetailPayload,
diff --git
a/airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py
b/airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py
index 4e848d4c3b9..1731bae3fe2 100644
---
a/airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py
+++
b/airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py
@@ -128,6 +128,10 @@ tracer = trace.get_tracer(__name__)
(status.HTTP_404_NOT_FOUND, "Task Instance not found"),
(status.HTTP_409_CONFLICT, "The TI is already in the requested
state"),
(HTTP_422_UNPROCESSABLE_CONTENT, "Invalid payload for the state
transition"),
+ (
+ status.HTTP_500_INTERNAL_SERVER_ERROR,
+ "The serialized TaskFlow arg spec for this stub task is not
valid",
+ ),
]
),
response_model_exclude_unset=True,
diff --git a/airflow-core/src/airflow/api_fastapi/execution_api/routes/xcoms.py
b/airflow-core/src/airflow/api_fastapi/execution_api/routes/xcoms.py
index 40469b3e56e..08b773076eb 100644
--- a/airflow-core/src/airflow/api_fastapi/execution_api/routes/xcoms.py
+++ b/airflow-core/src/airflow/api_fastapi/execution_api/routes/xcoms.py
@@ -27,6 +27,7 @@ from sqlalchemy.sql.selectable import Select
from airflow.api_fastapi.common.db.common import SessionDep
from airflow.api_fastapi.core_api.base import BaseModel
+from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
from airflow.api_fastapi.execution_api.datamodels.xcom import (
XComResponse,
XComSequenceIndexResponse,
@@ -268,6 +269,9 @@ def get_mapped_xcom_by_slice(
@router.head(
"/{dag_id}/{run_id}/{task_id}/{key:path}",
responses={
+ **create_openapi_http_exception_doc(
+ [(status.HTTP_400_BAD_REQUEST, "map_index cannot be specified in a
HEAD request")]
+ ),
status.HTTP_200_OK: {
"description": "Metadata about the number of matching XCom values",
"headers": {
@@ -366,6 +370,14 @@ def get_xcom(
@router.post(
"/{dag_id}/{run_id}/{task_id}/{key:path}",
status_code=status.HTTP_201_CREATED,
+ responses=create_openapi_http_exception_doc(
+ [
+ (
+ status.HTTP_400_BAD_REQUEST,
+ "The key is empty, the value is too large to map, or is
unserializable",
+ )
+ ]
+ ),
)
def set_xcom(
dag_id: str,
diff --git a/airflow-core/src/airflow/ui/openapi-gen/requests/services.gen.ts
b/airflow-core/src/airflow/ui/openapi-gen/requests/services.gen.ts
index 98bd10aa5a8..4989b4492ee 100644
--- a/airflow-core/src/airflow/ui/openapi-gen/requests/services.gen.ts
+++ b/airflow-core/src/airflow/ui/openapi-gen/requests/services.gen.ts
@@ -473,6 +473,7 @@ export class AssetService {
dag_id: data.dagId
},
errors: {
+ 404: 'Not Found',
422: 'Validation Error'
}
});
@@ -983,6 +984,7 @@ export class ConnectionService {
body: data.requestBody,
mediaType: 'application/json',
errors: {
+ 400: 'Bad Request',
401: 'Unauthorized',
403: 'Forbidden',
422: 'Validation Error'
@@ -3350,6 +3352,7 @@ export class TaskInstanceService {
token: data.token
},
errors: {
+ 400: 'Bad Request',
401: 'Unauthorized',
403: 'Forbidden',
404: 'Not Found',
@@ -4679,6 +4682,7 @@ export class VariableService {
errors: {
401: 'Unauthorized',
403: 'Forbidden',
+ 404: 'Not Found',
409: 'Conflict',
422: 'Validation Error'
}
@@ -4975,6 +4979,7 @@ export class PartitionedDagRunService {
partition_key: data.partitionKey
},
errors: {
+ 404: 'Not Found',
422: 'Validation Error'
}
});
@@ -5448,6 +5453,7 @@ export class TeamsService {
order_by: data.orderBy
},
errors: {
+ 403: 'Forbidden',
422: 'Validation Error'
}
});
diff --git a/airflow-core/src/airflow/ui/openapi-gen/requests/types.gen.ts
b/airflow-core/src/airflow/ui/openapi-gen/requests/types.gen.ts
index 441d1e1eb55..f867e01ee53 100644
--- a/airflow-core/src/airflow/ui/openapi-gen/requests/types.gen.ts
+++ b/airflow-core/src/airflow/ui/openapi-gen/requests/types.gen.ts
@@ -5526,6 +5526,10 @@ export type $OpenApiTs = {
* Successful Response
*/
200: NextRunAssetsResponse;
+ /**
+ * Not Found
+ */
+ 404: HTTPExceptionResponse;
/**
* Validation Error
*/
@@ -6012,6 +6016,10 @@ export type $OpenApiTs = {
* Successful Response
*/
200: ConnectionTestResponse;
+ /**
+ * Bad Request
+ */
+ 400: HTTPExceptionResponse;
/**
* Unauthorized
*/
@@ -7673,6 +7681,10 @@ export type $OpenApiTs = {
* Successful Response
*/
200: TaskInstancesLogResponse;
+ /**
+ * Bad Request
+ */
+ 400: HTTPExceptionResponse;
/**
* Unauthorized
*/
@@ -8740,6 +8752,10 @@ export type $OpenApiTs = {
* Forbidden
*/
403: HTTPExceptionResponse;
+ /**
+ * Not Found
+ */
+ 404: HTTPExceptionResponse;
/**
* Conflict
*/
@@ -8972,6 +8988,10 @@ export type $OpenApiTs = {
* Successful Response
*/
200: PartitionedDagRunDetailResponse;
+ /**
+ * Not Found
+ */
+ 404: HTTPExceptionResponse;
/**
* Validation Error
*/
@@ -9218,6 +9238,10 @@ export type $OpenApiTs = {
* Successful Response
*/
200: TeamCollectionResponse;
+ /**
+ * Forbidden
+ */
+ 403: HTTPExceptionResponse;
/**
* Validation Error
*/
diff --git a/scripts/ci/prek/check_openapi_exception_doc_in_sync.py
b/scripts/ci/prek/check_openapi_exception_doc_in_sync.py
new file mode 100755
index 00000000000..fe9aad01e01
--- /dev/null
+++ b/scripts/ci/prek/check_openapi_exception_doc_in_sync.py
@@ -0,0 +1,239 @@
+#!/usr/bin/env python
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Check API route handlers declare every HTTP status they raise.
+
+``responses=`` is what the generated OpenAPI spec — and every client built from
+it — uses to model error responses, but nothing ties it to the statuses a
+handler actually raises. The two drift apart silently, and that drift has been
+patched by hand repeatedly (#67570, #67571, #70992, #71011).
+
+A handler violates the rule when it raises ``HTTPException(<status>)`` in its
+own body with a status neither its own ``responses=`` block nor its router's
+declares. The check is deliberately conservative so it can gate CI: ``401``,
+``403`` and ``422`` are never required (FastAPI and the routers' auth
+dependencies supply them), only the handler's own body is inspected, and
+anything it cannot resolve statically is skipped rather than guessed at. It
+therefore under-reports rather than over-reports.
+"""
+
+# /// script
+# requires-python = ">=3.10,<3.11"
+# dependencies = [
+# "rich>=13.6.0",
+# ]
+# ///
+from __future__ import annotations
+
+import argparse
+import ast
+import re
+import sys
+from pathlib import Path
+
+from common_prek_utils import console
+
+ROUTE_METHODS = {"get", "post", "put", "patch", "delete", "head", "options"}
+ROUTER_CLASSES = {"APIRouter", "AirflowRouter", "VersionedAPIRouter"}
+DOC_HELPER = "create_openapi_http_exception_doc"
+# 422 is added to every route by FastAPI itself; 401/403 come from the
router's auth
+# dependencies, declared once on a router this file-scoped check often cannot
reach.
+ALWAYS_DOCUMENTED = {401, 403, 422}
+
+_STATUS_CONSTANT = re.compile(r"^HTTP_(\d{3})_")
+
+
+def _resolve_status(node: ast.expr) -> int | None:
+ """Resolve a status code expression to its numeric value, or None if
unknown."""
+ if isinstance(node, ast.Attribute):
+ name = node.attr
+ elif isinstance(node, ast.Name):
+ name = node.id
+ elif isinstance(node, ast.Constant) and isinstance(node.value, int):
+ return node.value
+ else:
+ return None
+ match = _STATUS_CONSTANT.match(name)
+ return int(match.group(1)) if match else None
+
+
+def _statuses_from_responses(responses: ast.expr) -> set[int] | None:
+ """Resolve a ``responses=`` value to its statuses, or None when
unanalyzable."""
+ # Routes that document a success body spell it as a mapping that unpacks
the helper
+ # alongside literal entries; routers use a plain ``{status:
{"description": ...}}``.
+ if isinstance(responses, ast.Dict):
+ collected: set[int] = set()
+ for key, value in zip(responses.keys, responses.values):
+ if key is None:
+ # A ``None`` key is a ``**`` unpacking; its value carries the
real statuses.
+ unpacked = _statuses_from_responses(value)
+ if unpacked is None:
+ return None
+ collected |= unpacked
+ else:
+ status = _resolve_status(key)
+ if status is None:
+ return None
+ collected.add(status)
+ return collected
+
+ if not (
+ isinstance(responses, ast.Call)
+ and isinstance(responses.func, ast.Name)
+ and responses.func.id == DOC_HELPER
+ and responses.args
+ and isinstance(entries := responses.args[0], (ast.List, ast.Tuple))
+ ):
+ return None
+
+ declared: set[int] = set()
+ for entry in entries.elts:
+ # Entries are either a bare status or a ``(status, description)`` pair.
+ target = entry.elts[0] if isinstance(entry, ast.Tuple) and entry.elts
else entry
+ status = _resolve_status(target)
+ if status is None:
+ return None
+ declared.add(status)
+ return declared
+
+
+def _declared_statuses(decorator: ast.Call) -> set[int] | None:
+ """Return statuses declared by the route's own ``responses=``."""
+ responses = next((kw.value for kw in decorator.keywords if kw.arg ==
"responses"), None)
+ return set() if responses is None else _statuses_from_responses(responses)
+
+
+def _router_statuses(tree: ast.Module) -> dict[str, set[int] | None]:
+ """Map each router built in this module to the statuses it declares for
every route on it."""
+ routers: dict[str, set[int] | None] = {}
+ for node in ast.walk(tree):
+ if not isinstance(node, ast.Assign):
+ continue
+ call = node.value
+ if not (
+ isinstance(call, ast.Call) and isinstance(call.func, ast.Name) and
call.func.id in ROUTER_CLASSES
+ ):
+ continue
+ responses = next((kw.value for kw in call.keywords if kw.arg ==
"responses"), None)
+ statuses = set() if responses is None else
_statuses_from_responses(responses)
+ for target in node.targets:
+ if isinstance(target, ast.Name):
+ routers[target.id] = statuses
+ return routers
+
+
+def _raised_statuses(handler: ast.FunctionDef | ast.AsyncFunctionDef) ->
dict[int, int]:
+ """Map each status raised as ``HTTPException`` in the body to its first
line."""
+ raised: dict[int, int] = {}
+ for node in ast.walk(handler):
+ if not (isinstance(node, ast.Call) and isinstance(node.func,
ast.Name)):
+ continue
+ if node.func.id != "HTTPException":
+ continue
+ argument = next(
+ (kw.value for kw in node.keywords if kw.arg == "status_code"),
+ node.args[0] if node.args else None,
+ )
+ if argument is None:
+ continue
+ if (status := _resolve_status(argument)) is not None:
+ raised.setdefault(status, node.lineno)
+ return raised
+
+
+def _route_decorators(handler: ast.FunctionDef | ast.AsyncFunctionDef) ->
list[ast.Call]:
+ return [
+ decorator
+ for decorator in handler.decorator_list
+ if isinstance(decorator, ast.Call)
+ and isinstance(decorator.func, ast.Attribute)
+ and decorator.func.attr in ROUTE_METHODS
+ ]
+
+
+def check_file(file_path: Path) -> list[tuple[str, int, int]]:
+ """Return ``(handler_name, status, line_number)`` for each undeclared
status."""
+ try:
+ tree = ast.parse(file_path.read_text(encoding="utf-8"),
filename=str(file_path))
+ except (OSError, UnicodeDecodeError, SyntaxError):
+ return []
+
+ routers = _router_statuses(tree)
+ violations: list[tuple[str, int, int]] = []
+ for handler in ast.walk(tree):
+ if not isinstance(handler, (ast.FunctionDef, ast.AsyncFunctionDef)):
+ continue
+ for decorator in _route_decorators(handler):
+ declared = _declared_statuses(decorator)
+ # A route inherits whatever its router declares for every route on
it.
+ router = decorator.func.value if isinstance(decorator.func,
ast.Attribute) else None
+ inherited = routers.get(router.id, set()) if isinstance(router,
ast.Name) else set()
+ if declared is None or inherited is None:
+ continue
+ undeclared = {
+ status: lineno
+ for status, lineno in _raised_statuses(handler).items()
+ if status not in declared | inherited | ALWAYS_DOCUMENTED
+ }
+ violations.extend((handler.name, status, lineno) for status,
lineno in sorted(undeclared.items()))
+ return violations
+
+
+def main() -> int:
+ parser = argparse.ArgumentParser(description="Check API routes declare the
statuses they raise")
+ parser.add_argument("files", nargs="*", help="Files to check")
+ args = parser.parse_args()
+
+ total = 0
+ for file_path in (Path(f) for f in args.files):
+ violations = check_file(file_path)
+ if not violations:
+ continue
+ total += len(violations)
+ lines = [
+ f" Line {lineno}: {handler}() raises {status} but never declares
it"
+ for handler, status, lineno in violations
+ ]
+ if console:
+ console.print(f"[red]{file_path}[/red]:")
+ for line in lines:
+ console.print(f"[yellow]{line}[/yellow]")
+ else:
+ print(f"{file_path}:")
+ print("\n".join(lines))
+
+ if total:
+ message = (
+ f"Found {total} HTTP status(es) raised by a route handler but
missing from its "
+ f"`responses=` block.\n"
+ f"Add each one to `{DOC_HELPER}([...])` on the route decorator so
the generated "
+ "OpenAPI spec — and the clients generated from it — model the
response the API "
+ "really returns."
+ )
+ if console:
+ console.print()
+ console.print(f"[red]{message}[/red]")
+ else:
+ print()
+ print(message)
+ return 1
+ return 0
+
+
+if __name__ == "__main__":
+ sys.exit(main())
diff --git a/scripts/tests/ci/prek/test_check_openapi_exception_doc_in_sync.py
b/scripts/tests/ci/prek/test_check_openapi_exception_doc_in_sync.py
new file mode 100644
index 00000000000..ef9adc6bb20
--- /dev/null
+++ b/scripts/tests/ci/prek/test_check_openapi_exception_doc_in_sync.py
@@ -0,0 +1,335 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from pathlib import Path
+
+import pytest
+from check_openapi_exception_doc_in_sync import check_file
+
+
+class TestCheckFile:
+ @pytest.mark.parametrize(
+ "code, expected",
+ [
+ pytest.param(
+ """
+ @router.get("/x")
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ [("handler", 404, 4)],
+ id="no-responses-block-at-all",
+ ),
+ pytest.param(
+ """
+ @router.get(
+ "/x",
+
responses=create_openapi_http_exception_doc([status.HTTP_409_CONFLICT]),
+ )
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ [("handler", 404, 7)],
+ id="status-missing-from-responses",
+ ),
+ pytest.param(
+ """
+ @router.post("/x")
+ def handler():
+ if a:
+ raise
HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="bad")
+ raise HTTPException(status_code=status.HTTP_409_CONFLICT,
detail="taken")
+ """,
+ [("handler", 400, 5), ("handler", 409, 6)],
+ id="several-undeclared-statuses-are-all-reported",
+ ),
+ pytest.param(
+ """
+ @router.get("/x")
+ def handler():
+ raise HTTPException(HTTP_404_NOT_FOUND, "nope")
+ """,
+ [("handler", 404, 4)],
+ id="bare-status-constant",
+ ),
+ pytest.param(
+ """
+ @router.get("/x")
+ def handler():
+ raise HTTPException(404, "nope")
+ """,
+ [("handler", 404, 4)],
+ id="literal-status-code",
+ ),
+ pytest.param(
+ """
+ @router.get("/x")
+ async def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ [("handler", 404, 4)],
+ id="async-handler",
+ ),
+ pytest.param(
+ """
+ @router.get("/x")
+ def handler():
+ def fail():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ fail()
+ """,
+ [("handler", 404, 5)],
+ id="raise-nested-inside-handler",
+ ),
+ pytest.param(
+ """
+ @router.delete(
+ "/x",
+ responses=create_openapi_http_exception_doc(
+ [(status.HTTP_409_CONFLICT, "conflict")]
+ ),
+ )
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ [("handler", 404, 9)],
+ id="tuple-form-declares-a-different-status",
+ ),
+ pytest.param(
+ """
+ router = APIRouter(
+ responses={status.HTTP_409_CONFLICT: {"description":
"conflict"}},
+ )
+
+ @router.get("/x")
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ [("handler", 404, 8)],
+ id="router-declares-a-different-status",
+ ),
+ pytest.param(
+ """
+ @router.get(
+ "/x",
+ responses={
+ status.HTTP_200_OK: {"description": "ok"},
+ },
+ )
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ [("handler", 404, 9)],
+ id="mapping-responses-without-the-status",
+ ),
+ ],
+ )
+ def test_violations_detected(self, write_python_file, code: str, expected):
+ assert check_file(write_python_file(code)) == expected
+
+ @pytest.mark.parametrize(
+ "code",
+ [
+ pytest.param(
+ """
+ @router.get(
+ "/x",
+
responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+ )
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ id="status-declared-plainly",
+ ),
+ pytest.param(
+ """
+ @router.get(
+ "/x",
+ responses=create_openapi_http_exception_doc(
+ [(status.HTTP_404_NOT_FOUND, "not found")]
+ ),
+ )
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ id="status-declared-with-description",
+ ),
+ pytest.param(
+ """
+ @router.get("/x")
+ def handler():
+ raise HTTPException(status.HTTP_422_UNPROCESSABLE_CONTENT,
"invalid")
+ """,
+ id="422-is-documented-by-fastapi-itself",
+ ),
+ pytest.param(
+ """
+ @router.get("/x")
+ def handler():
+ raise HTTPException(status.HTTP_401_UNAUTHORIZED, "who?")
+ """,
+ id="401-is-declared-on-the-router",
+ ),
+ pytest.param(
+ """
+ @router.get("/x")
+ def handler():
+ raise HTTPException(status.HTTP_403_FORBIDDEN, "no")
+ """,
+ id="403-is-declared-on-the-router",
+ ),
+ pytest.param(
+ """
+ router = APIRouter(
+ responses={status.HTTP_404_NOT_FOUND: {"description": "not
found"}},
+ )
+
+ @router.get("/x")
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ id="router-declares-the-status-for-every-route",
+ ),
+ pytest.param(
+ """
+ router = VersionedAPIRouter(
+ responses={status.HTTP_404_NOT_FOUND: {"description": "not
found"}},
+ )
+
+ @router.get("/x")
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ id="cadwyn-versioned-router-declares-the-status",
+ ),
+ pytest.param(
+ """
+ teams_router = AirflowRouter()
+
+ @teams_router.get(
+ "/x",
+
responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+ )
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ id="airflow-router-without-shared-responses",
+ ),
+ pytest.param(
+ """
+ @router.get(
+ "/x",
+ responses={
+
**create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+ status.HTTP_200_OK: {"description": "ok"},
+ },
+ )
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ id="helper-unpacked-into-a-mapping-alongside-a-success-body",
+ ),
+ pytest.param(
+ """
+ router = APIRouter(responses=SHARED_ERRORS)
+
+ @router.get("/x")
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ id="router-responses-cannot-be-resolved",
+ ),
+ pytest.param(
+ """
+ @router.get(
+ "/x",
+ responses={SOME_ALIAS: {"description": "?"}},
+ )
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ id="mapping-key-cannot-be-resolved",
+ ),
+ pytest.param(
+ """
+ @router.get("/x", responses={404: {"model": Foo}})
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ id="unrecognised-responses-shape-is-skipped",
+ ),
+ pytest.param(
+ """
+ @router.get("/x",
responses=create_openapi_http_exception_doc(SHARED_ERRORS))
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ id="declared-list-is-not-a-literal",
+ ),
+ pytest.param(
+ """
+ @router.get(
+ "/x",
+ responses=create_openapi_http_exception_doc([SOME_ALIAS]),
+ )
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ id="declared-entry-cannot-be-resolved",
+ ),
+ pytest.param(
+ """
+ @router.get("/x")
+ def handler():
+ raise HTTPException(chosen_status, "nope")
+ """,
+ id="raised-status-cannot-be-resolved",
+ ),
+ pytest.param(
+ """
+ def helper():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ id="not-a-route-handler",
+ ),
+ pytest.param(
+ """
+ @router.websocket("/x")
+ def handler():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+ """,
+ id="not-an-http-route-method",
+ ),
+ pytest.param(
+ """
+ @router.get("/x")
+ def handler():
+ return 1
+ """,
+ id="handler-raises-nothing",
+ ),
+ ],
+ )
+ def test_no_violation(self, write_python_file, code: str):
+ assert check_file(write_python_file(code)) == []
+
+ def test_syntax_error_is_silently_skipped(self, write_python_file):
+ assert check_file(write_python_file("def broken(:\n")) == []
+
+ def test_missing_file_is_silently_skipped(self, tmp_path: Path):
+ assert check_file(tmp_path / "does_not_exist.py") == []
