This is an automated email from the ASF dual-hosted git repository.
pierrejeambrun pushed a commit to branch v3-3-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/v3-3-test by this push:
new 1fa2bbbfc41 Keep OpenAPI error responses in sync with the statuses
routes raise (#71647) (#73968)
1fa2bbbfc41 is described below
commit 1fa2bbbfc41e285a48464f98d14b259136931a62
Author: Pierre Jeambrun <[email protected]>
AuthorDate: Thu Oct 1 10:55:57 2026 +0200
Keep OpenAPI error responses in sync with the statuses routes raise
(#71647) (#73968)
* 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.
(cherry picked from commit 8d4ab8a94db291a359b686ac545211f4c440cb3e)
Co-authored-by: Pushkal Gupta
<[email protected]>
---
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 0d2fa10c1d8..ac20ef1357b 100644
--- a/airflow-core/.pre-commit-config.yaml
+++ b/airflow-core/.pre-commit-config.yaml
@@ -159,6 +159,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 05565913372..77bf194d70c 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
@@ -104,6 +104,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:
@@ -229,6 +235,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:
@@ -1701,6 +1713,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 f7e902b1793..9344bc1d9d5 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
@@ -1866,6 +1866,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:
@@ -9701,6 +9707,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:
@@ -9849,6 +9861,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 e828b932dea..5df9815d637 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
@@ -324,7 +324,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 4c0e34fd297..d621d8c0aed 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
@@ -29,6 +29,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.security import requires_access_asset,
requires_access_dag
from airflow.models import DagModel
from airflow.models.asset import (
@@ -51,6 +52,7 @@ assets_router = AirflowRouter(tags=["Asset"])
@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 959f86c8335..a1ab8f6a700 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
@@ -351,6 +351,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 d0ecc3d3ada..eac8e1cd443 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
@@ -24,6 +24,7 @@ from sqlalchemy import and_, select
from airflow.api_fastapi.common.db.common import SessionDep
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,
@@ -73,7 +74,12 @@ def _get_asset_events_through_sql_clauses(
)
[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 74cda44efe7..8d9b567feea 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,
@@ -109,7 +110,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 a7eef46571b..b1e28729bef 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
@@ -120,6 +120,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 2e1736a5290..487d3189206 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,
@@ -263,6 +264,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": {
@@ -361,6 +365,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 a2e192fa250..01a85a9bdd0 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
@@ -414,6 +414,7 @@ export class AssetService {
dag_id: data.dagId
},
errors: {
+ 404: 'Not Found',
422: 'Validation Error'
}
});
@@ -850,6 +851,7 @@ export class ConnectionService {
body: data.requestBody,
mediaType: 'application/json',
errors: {
+ 400: 'Bad Request',
401: 'Unauthorized',
403: 'Forbidden',
422: 'Validation Error'
@@ -3023,6 +3025,7 @@ export class TaskInstanceService {
token: data.token
},
errors: {
+ 400: 'Bad Request',
401: 'Unauthorized',
403: 'Forbidden',
404: 'Not Found',
@@ -4344,6 +4347,7 @@ export class VariableService {
errors: {
401: 'Unauthorized',
403: 'Forbidden',
+ 404: 'Not Found',
409: 'Conflict',
422: 'Validation Error'
}
@@ -4636,6 +4640,7 @@ export class PartitionedDagRunService {
partition_key: data.partitionKey
},
errors: {
+ 404: 'Not Found',
422: 'Validation Error'
}
});
@@ -5069,6 +5074,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 2fee8a825a2..bb326638cd7 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
@@ -4935,6 +4935,10 @@ export type $OpenApiTs = {
* Successful Response
*/
200: NextRunAssetsResponse;
+ /**
+ * Not Found
+ */
+ 404: HTTPExceptionResponse;
/**
* Validation Error
*/
@@ -5394,6 +5398,10 @@ export type $OpenApiTs = {
* Successful Response
*/
200: ConnectionTestResponse;
+ /**
+ * Bad Request
+ */
+ 400: HTTPExceptionResponse;
/**
* Unauthorized
*/
@@ -6921,6 +6929,10 @@ export type $OpenApiTs = {
* Successful Response
*/
200: TaskInstancesLogResponse;
+ /**
+ * Bad Request
+ */
+ 400: HTTPExceptionResponse;
/**
* Unauthorized
*/
@@ -7988,6 +8000,10 @@ export type $OpenApiTs = {
* Forbidden
*/
403: HTTPExceptionResponse;
+ /**
+ * Not Found
+ */
+ 404: HTTPExceptionResponse;
/**
* Conflict
*/
@@ -8220,6 +8236,10 @@ export type $OpenApiTs = {
* Successful Response
*/
200: PartitionedDagRunDetailResponse;
+ /**
+ * Not Found
+ */
+ 404: HTTPExceptionResponse;
/**
* Validation Error
*/
@@ -8455,6 +8475,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") == []
