This is an automated email from the ASF dual-hosted git repository.
bbovenzi 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 4da2ad1e2b3 Document HTTP statuses that route helpers raise (#74027)
4da2ad1e2b3 is described below
commit 4da2ad1e2b3dee214d564a2b365da8986e9918aa
Author: Pushkal Gupta <[email protected]>
AuthorDate: Thu Oct 8 01:45:22 2026 +0530
Document HTTP statuses that route helpers raise (#74027)
A route's `responses=` block is the only thing the generated OpenAPI spec
and
its clients have to model error responses, but the check added in #71647
only
looked at statuses a handler raised directly. Most route code reaches its
error
paths through a helper, so the statuses that matter most to a client were
the
ones the check could not see, and six of them were undeclared across the
public,
UI and execution APIs.
Calls are followed from a handler's body only. A route decorator's
`Depends(...)`
security dependencies resolve to functions that raise 400 on an invalid
identifier, and those statuses come from the router rather than the route,
so
following them would report a status the route is not responsible for
declaring.
Co-authored-by: Pierre Jeambrun <[email protected]>
---
.../api_fastapi/core_api/openapi/_private_ui.yaml | 6 +
.../core_api/openapi/v2-rest-api-generated.yaml | 6 +
.../core_api/routes/public/task_state_store.py | 2 +-
.../api_fastapi/core_api/routes/ui/calendar.py | 4 +-
.../api_fastapi/execution_api/routes/hitl.py | 6 +-
.../execution_api/routes/task_instances.py | 12 +-
.../ui/openapi-gen/requests/services.gen.ts | 2 +
.../airflow/ui/openapi-gen/requests/types.gen.ts | 8 +
.../ci/prek/check_openapi_exception_doc_in_sync.py | 201 ++++++++++++++++++---
.../test_check_openapi_exception_doc_in_sync.py | 114 +++++++++++-
10 files changed, 321 insertions(+), 40 deletions(-)
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 3cf4d8866d2..cfcabb0d409 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
@@ -2384,6 +2384,12 @@ paths:
application/json:
schema:
$ref:
'#/components/schemas/CalendarTimeRangeCollectionResponse'
+ '404':
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/HTTPExceptionResponse'
+ description: Not Found
'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 671ac401abc..ba08ddd4fe6 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
@@ -6603,6 +6603,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/task_state_store.py
b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/task_state_store.py
index 48699605ad5..71780415bd8 100644
---
a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/task_state_store.py
+++
b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/task_state_store.py
@@ -221,7 +221,7 @@ def get_task_state_store(
@task_state_store_router.put(
"/{key:path}",
status_code=status.HTTP_204_NO_CONTENT,
- responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+ responses=create_openapi_http_exception_doc([status.HTTP_400_BAD_REQUEST,
status.HTTP_404_NOT_FOUND]),
dependencies=[Depends(requires_access_dag(method="PUT",
access_entity=DagAccessEntity.TASK_INSTANCE))],
)
def set_task_state_store(
diff --git
a/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/calendar.py
b/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/calendar.py
index 066a4845b82..3afc5fb5e9d 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/calendar.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/ui/calendar.py
@@ -18,7 +18,7 @@ from __future__ import annotations
from typing import Annotated, Literal
-from fastapi import Depends
+from fastapi import Depends, status
from airflow.api_fastapi.auth.managers.models.resource_details import
DagAccessEntity
from airflow.api_fastapi.common.dagbag import DagBagDep,
get_latest_version_of_dag
@@ -29,6 +29,7 @@ from airflow.api_fastapi.core_api.datamodels.ui.calendar
import (
CalendarDeadlineCollectionResponse,
CalendarTimeRangeCollectionResponse,
)
+from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
from airflow.api_fastapi.core_api.security import requires_access_dag
from airflow.api_fastapi.core_api.services.ui.calendar import CalendarService
from airflow.models.dagrun import DagRun
@@ -39,6 +40,7 @@ calendar_router = AirflowRouter(prefix="/calendar",
tags=["Calendar"])
@calendar_router.get(
"/{dag_id}",
+ responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
dependencies=[
Depends(
requires_access_dag(
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 395a3ea30e8..e27349684e3 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
@@ -123,7 +123,10 @@ def _check_hitl_detail_exists(hitl_detail_model:
HITLDetail | None) -> HITLDetai
@router.patch(
"/{task_instance_id}",
responses=create_openapi_http_exception_doc(
- [(status.HTTP_409_CONFLICT, "A response has already been received for
this HITLDetail")]
+ [
+ (status.HTTP_404_NOT_FOUND, "HITLDetail not found"),
+ (status.HTTP_409_CONFLICT, "A response has already been received
for this HITLDetail"),
+ ]
),
)
def update_hitl_detail(
@@ -153,6 +156,7 @@ def update_hitl_detail(
@router.get(
"/{task_instance_id}",
status_code=status.HTTP_200_OK,
+ responses=create_openapi_http_exception_doc([(status.HTTP_404_NOT_FOUND,
"HITLDetail not found")]),
)
async def get_hitl_detail(
task_instance_id: UUID,
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 1525be1c4bc..d5606864cd8 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
@@ -1257,7 +1257,11 @@ async def get_previous_successful_dagrun(
return PrevSuccessfulDagRunResponse.model_validate(dag_run)
[email protected]("/count", status_code=status.HTTP_200_OK)
[email protected](
+ "/count",
+ status_code=status.HTTP_200_OK,
+ responses=create_openapi_http_exception_doc([(status.HTTP_404_NOT_FOUND,
"Task group not found")]),
+)
def get_task_instance_count(
dag_id: str,
session: SessionDep,
@@ -1366,7 +1370,11 @@ async def get_previous_task_instance(
)
[email protected]("/states", status_code=status.HTTP_200_OK)
[email protected](
+ "/states",
+ status_code=status.HTTP_200_OK,
+ responses=create_openapi_http_exception_doc([(status.HTTP_404_NOT_FOUND,
"Task group not found")]),
+)
def get_task_instance_states(
dag_id: str,
session: SessionDep,
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 df90ed6a832..1c89adc1f8d 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
@@ -4297,6 +4297,7 @@ export class TaskStateStoreService {
body: data.requestBody,
mediaType: 'application/json',
errors: {
+ 400: 'Bad Request',
401: 'Unauthorized',
403: 'Forbidden',
404: 'Not Found',
@@ -5535,6 +5536,7 @@ export class CalendarService {
partition_date_lt: data.partitionDateLt
},
errors: {
+ 404: 'Not Found',
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 e6bea43bda0..490a2c00d27 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
@@ -8595,6 +8595,10 @@ export type $OpenApiTs = {
* Successful Response
*/
204: void;
+ /**
+ * Bad Request
+ */
+ 400: HTTPExceptionResponse;
/**
* Unauthorized
*/
@@ -9453,6 +9457,10 @@ export type $OpenApiTs = {
* Successful Response
*/
200: CalendarTimeRangeCollectionResponse;
+ /**
+ * Not Found
+ */
+ 404: 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
index e4f2dea46ba..409f0d67487 100755
--- a/scripts/ci/prek/check_openapi_exception_doc_in_sync.py
+++ b/scripts/ci/prek/check_openapi_exception_doc_in_sync.py
@@ -23,13 +23,18 @@ 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.
+A handler violates the rule when it raises ``HTTPException(<status>)`` -- in
its
+own body, or in a helper it calls -- with a status neither its own
``responses=``
+block nor its router's declares. Helper calls are followed across modules up to
+``MAX_CALL_DEPTH``, which is how a status raised by something like
+``get_latest_version_of_dag`` is attributed to the route that calls it.
+
+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 calls in a handler's *body* are followed, so the security
+dependencies in a route decorator stay exempt, and anything that cannot be
+resolved statically is skipped rather than guessed at. It therefore
+under-reports rather than over-reports.
"""
# /// script
@@ -45,6 +50,7 @@ import ast
import re
import sys
from pathlib import Path
+from typing import NamedTuple
from common_prek_utils import console
@@ -55,9 +61,24 @@ DOC_HELPER = "create_openapi_http_exception_doc"
# dependencies, declared once on a router this file-scoped check often cannot
reach.
ALWAYS_DOCUMENTED = {401, 403, 422}
+# Helper chains in these routes are shallow; the bound only stops pathological
recursion.
+MAX_CALL_DEPTH = 3
+
_STATUS_CONSTANT = re.compile(r"^HTTP_(\d{3})_")
+class Violation(NamedTuple):
+ handler: str
+ status: int
+ lineno: int
+ # Name of the helper that raises the status, or None when the handler
raises it itself.
+ via: str | None
+
+ def describe(self) -> str:
+ source = f" via {self.via}()" if self.via else ""
+ return f" Line {self.lineno}: {self.handler}() raises
{self.status}{source} but never declares it"
+
+
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):
@@ -137,13 +158,23 @@ def _router_statuses(tree: ast.Module) -> dict[str,
set[int] | None]:
return routers
-def _raised_statuses(handler: ast.FunctionDef | ast.AsyncFunctionDef) ->
dict[int, int]:
+FunctionNode = ast.FunctionDef | ast.AsyncFunctionDef
+
+
+def _body_calls(function: FunctionNode) -> list[ast.Call]:
+ """Return calls made in the function's body, excluding its decorators.
+
+ Route decorators carry the ``Depends(...)`` security dependencies, whose
statuses
+ the router supplies and this check does not require, so they must not be
followed.
+ """
+ return [node for statement in function.body for node in
ast.walk(statement) if isinstance(node, ast.Call)]
+
+
+def _raised_statuses(handler: FunctionNode) -> 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":
+ for node in _body_calls(handler):
+ if not isinstance(node.func, ast.Name) or node.func.id !=
"HTTPException":
continue
argument = next(
(kw.value for kw in node.keywords if kw.arg == "status_code"),
@@ -156,7 +187,123 @@ def _raised_statuses(handler: ast.FunctionDef |
ast.AsyncFunctionDef) -> dict[in
return raised
-def _route_decorators(handler: ast.FunctionDef | ast.AsyncFunctionDef) ->
list[ast.Call]:
+def _source_root(file_path: Path) -> Path | None:
+ """Return the import root (the ``src`` directory) this file lives under."""
+ return next((parent for parent in file_path.parents if parent.name ==
"src"), None)
+
+
+class _ModuleIndex:
+ """Lazily parsed view of the functions each module defines, keyed by
import path."""
+
+ def __init__(self, source_root: Path | None) -> None:
+ self._source_root = source_root
+ self._cache: dict[str, dict[str, FunctionNode]] = {}
+
+ def functions(self, module: str) -> dict[str, FunctionNode]:
+ if (source_root := self._source_root) is None:
+ return {}
+ if module not in self._cache:
+ self._cache[module] = self._parse(source_root, module)
+ return self._cache[module]
+
+ @staticmethod
+ def _parse(source_root: Path, module: str) -> dict[str, FunctionNode]:
+ relative = Path(*module.split("."))
+ for candidate in (
+ source_root / relative.with_suffix(".py"),
+ source_root / relative / "__init__.py",
+ ):
+ try:
+ tree = ast.parse(candidate.read_text(encoding="utf-8"),
filename=str(candidate))
+ except (OSError, UnicodeDecodeError, SyntaxError):
+ continue
+ return {
+ node.name: node
+ for node in ast.walk(tree)
+ if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))
+ }
+ return {}
+
+
+def _imported_functions(tree: ast.Module) -> dict[str, tuple[str, str]]:
+ """Map each name bound by ``from <module> import <name>`` to ``(module,
original)``."""
+ imported: dict[str, tuple[str, str]] = {}
+ for node in ast.walk(tree):
+ # ``level`` is non-zero for relative imports, which this resolver does
not handle.
+ if isinstance(node, ast.ImportFrom) and node.module and not node.level:
+ for alias in node.names:
+ imported[alias.asname or alias.name] = (node.module,
alias.name)
+ return imported
+
+
+class _CallGraph:
+ """Resolves the statuses a handler raises through the helpers it calls."""
+
+ def __init__(self, tree: ast.Module, index: _ModuleIndex) -> None:
+ self._index = index
+ self._local = {
+ node.name: node
+ for node in ast.walk(tree)
+ if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))
+ }
+ self._imported = _imported_functions(tree)
+
+ def _resolve(
+ self, name: str, namespace: dict[str, FunctionNode]
+ ) -> tuple[FunctionNode, dict[str, FunctionNode]] | None:
+ """Return the called function and the namespace defining it.
+
+ Imports are followed only from the route module itself; inside an
imported
+ module just that module's own definitions are visible, which is what
keeps a
+ chain from fanning out across the whole package.
+ """
+ if (function := namespace.get(name)) is not None:
+ return function, namespace
+ if namespace is not self._local or (origin :=
self._imported.get(name)) is None:
+ return None
+ module, original = origin
+ imported = self._index.functions(module)
+ if (function := imported.get(original)) is None:
+ return None
+ return function, imported
+
+ def statuses_via_helpers(self, handler: FunctionNode) -> dict[int, str]:
+ """Map each status raised only by a helper to the name of the helper
raising it."""
+ found: dict[int, str] = {}
+ self._walk(handler, self._local, depth=0, seen={handler.name},
attribute_to=None, found=found)
+ for status in _raised_statuses(handler):
+ found.pop(status, None)
+ return found
+
+ def _walk(
+ self,
+ function: FunctionNode,
+ namespace: dict[str, FunctionNode],
+ depth: int,
+ seen: set[str],
+ attribute_to: str | None,
+ found: dict[int, str],
+ ) -> None:
+ if depth >= MAX_CALL_DEPTH:
+ return
+ for call in _body_calls(function):
+ if not isinstance(call.func, ast.Name):
+ continue
+ name = call.func.id
+ if name in seen:
+ continue
+ resolved = self._resolve(name, namespace)
+ if resolved is None:
+ continue
+ callee, callee_namespace = resolved
+ # The first helper in the chain is the one worth naming in the
message.
+ credit = attribute_to or name
+ for status in _raised_statuses(callee):
+ found.setdefault(status, credit)
+ self._walk(callee, callee_namespace, depth + 1, seen | {name},
credit, found)
+
+
+def _route_decorators(handler: FunctionNode) -> list[ast.Call]:
return [
decorator
for decorator in handler.decorator_list
@@ -166,15 +313,16 @@ def _route_decorators(handler: ast.FunctionDef |
ast.AsyncFunctionDef) -> list[a
]
-def check_file(file_path: Path) -> list[tuple[str, int, int]]:
- """Return ``(handler_name, status, line_number)`` for each undeclared
status."""
+def check_file(file_path: Path) -> list[Violation]:
+ """Return a :class:`Violation` for each status a route raises but never
declares."""
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]] = []
+ call_graph = _CallGraph(tree, _ModuleIndex(_source_root(file_path)))
+ violations: list[Violation] = []
for handler in ast.walk(tree):
if not isinstance(handler, (ast.FunctionDef, ast.AsyncFunctionDef)):
continue
@@ -185,12 +333,18 @@ def check_file(file_path: Path) -> list[tuple[str, int,
int]]:
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
+ documented = declared | inherited | ALWAYS_DOCUMENTED
+ found = [
+ Violation(handler.name, status, lineno, None)
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()))
+ if status not in documented
+ ]
+ found += [
+ Violation(handler.name, status, handler.lineno, helper)
+ for status, helper in
call_graph.statuses_via_helpers(handler).items()
+ if status not in documented
+ ]
+ violations.extend(sorted(found, key=lambda violation:
violation.status))
return violations
@@ -205,10 +359,7 @@ def main() -> int:
if not violations:
continue
total += len(violations)
- lines = [
- f" Line {lineno}: {handler}() raises {status} but never declares
it"
- for handler, status, lineno in violations
- ]
+ lines = [violation.describe() for violation in violations]
if console:
console.print(f"[red]{file_path}[/red]:")
for line in lines:
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
index ef9adc6bb20..2aaf69991ed 100644
--- 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
@@ -32,7 +32,7 @@ class TestCheckFile:
def handler():
raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
""",
- [("handler", 404, 4)],
+ [("handler", 404, 4, None)],
id="no-responses-block-at-all",
),
pytest.param(
@@ -44,7 +44,7 @@ class TestCheckFile:
def handler():
raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
""",
- [("handler", 404, 7)],
+ [("handler", 404, 7, None)],
id="status-missing-from-responses",
),
pytest.param(
@@ -55,7 +55,7 @@ class TestCheckFile:
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)],
+ [("handler", 400, 5, None), ("handler", 409, 6, None)],
id="several-undeclared-statuses-are-all-reported",
),
pytest.param(
@@ -64,7 +64,7 @@ class TestCheckFile:
def handler():
raise HTTPException(HTTP_404_NOT_FOUND, "nope")
""",
- [("handler", 404, 4)],
+ [("handler", 404, 4, None)],
id="bare-status-constant",
),
pytest.param(
@@ -73,7 +73,7 @@ class TestCheckFile:
def handler():
raise HTTPException(404, "nope")
""",
- [("handler", 404, 4)],
+ [("handler", 404, 4, None)],
id="literal-status-code",
),
pytest.param(
@@ -82,7 +82,7 @@ class TestCheckFile:
async def handler():
raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
""",
- [("handler", 404, 4)],
+ [("handler", 404, 4, None)],
id="async-handler",
),
pytest.param(
@@ -93,7 +93,7 @@ class TestCheckFile:
raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
fail()
""",
- [("handler", 404, 5)],
+ [("handler", 404, 5, None)],
id="raise-nested-inside-handler",
),
pytest.param(
@@ -107,7 +107,7 @@ class TestCheckFile:
def handler():
raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
""",
- [("handler", 404, 9)],
+ [("handler", 404, 9, None)],
id="tuple-form-declares-a-different-status",
),
pytest.param(
@@ -120,7 +120,7 @@ class TestCheckFile:
def handler():
raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
""",
- [("handler", 404, 8)],
+ [("handler", 404, 8, None)],
id="router-declares-a-different-status",
),
pytest.param(
@@ -134,7 +134,7 @@ class TestCheckFile:
def handler():
raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
""",
- [("handler", 404, 9)],
+ [("handler", 404, 9, None)],
id="mapping-responses-without-the-status",
),
],
@@ -142,6 +142,58 @@ class TestCheckFile:
def test_violations_detected(self, write_python_file, code: str, expected):
assert check_file(write_python_file(code)) == expected
+ @pytest.mark.parametrize(
+ "code, expected",
+ [
+ pytest.param(
+ """
+ def _find_it(value):
+ if value is None:
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+
+ @router.get("/x")
+ def handler():
+ return _find_it(None)
+ """,
+ [("handler", 404, 7, "_find_it")],
+ id="status-raised-by-a-same-module-helper",
+ ),
+ pytest.param(
+ """
+ def _inner():
+ raise HTTPException(status.HTTP_409_CONFLICT, "nope")
+
+ def _outer():
+ return _inner()
+
+ @router.get("/x")
+ def handler():
+ return _outer()
+ """,
+ [("handler", 409, 9, "_outer")],
+ id="helper-chain-is-followed-and-credited-to-the-first-hop",
+ ),
+ pytest.param(
+ """
+ def _find_it():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+
+ @router.get(
+ "/x",
+
responses=create_openapi_http_exception_doc([status.HTTP_400_BAD_REQUEST]),
+ )
+ def handler():
+ _find_it()
+ raise HTTPException(status.HTTP_500_INTERNAL_SERVER_ERROR,
"boom")
+ """,
+ [("handler", 404, 9, "_find_it"), ("handler", 500, 11, None)],
+ id="handler-and-helper-statuses-are-both-reported",
+ ),
+ ],
+ )
+ def test_statuses_raised_through_helpers(self, write_python_file, code:
str, expected):
+ assert sorted(check_file(write_python_file(code))) == sorted(expected)
+
@pytest.mark.parametrize(
"code",
[
@@ -217,6 +269,48 @@ class TestCheckFile:
""",
id="cadwyn-versioned-router-declares-the-status",
),
+ pytest.param(
+ """
+ def requires_access(method):
+ raise HTTPException(status.HTTP_400_BAD_REQUEST, "bad")
+
+ @router.get(
+ "/x",
+ dependencies=[Depends(requires_access(method="GET"))],
+ )
+ def handler():
+ return None
+ """,
+ id="security-dependency-in-the-decorator-is-not-followed",
+ ),
+ pytest.param(
+ """
+ def _helper():
+ raise HTTPException(status.HTTP_404_NOT_FOUND, "nope")
+
+ @router.get(
+ "/x",
+
responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+ )
+ def handler():
+ return _helper()
+ """,
+ id="helper-status-already-declared-on-the-route",
+ ),
+ pytest.param(
+ """
+ def _a():
+ return _b()
+
+ def _b():
+ return _a()
+
+ @router.get("/x")
+ def handler():
+ return _a()
+ """,
+ id="recursive-helpers-terminate",
+ ),
pytest.param(
"""
teams_router = AirflowRouter()
