AIP-84 Add HTTPException openapi documentation #42508
Conversation
pierrejeambrun
commented
Sep 26, 2024
•
pierrejeambrun
commented
pierrejeambrun
requested review from
bbovenzi,
ryanahamilton and
jscheffl
as code owners
|
cc: @jscheffl This will take care of adding the necessary documentation of possible errors to the openapi spec. |
|
|
||
|
|
||
| @dags_router.patch("/dags/{dag_id}") | ||
| @dags_router.patch("/dags/{dag_id}", responses=create_openapi_http_exception_doc([400, 401, 403, 404])) |
There was a problem hiding this comment.
This is how we tell FastAPI the possible error code returned by the endpoint
There was a problem hiding this comment.
Interesting that this is not natively supported. Was it that our previous approach was not according to design of FastAPI or is this missin gin the copcepts?
pierrejeambrun
force-pushed
the
document-http-errors-fastapi-api
branch
from
cb5bf69
to
f616187
Compare
There was a problem hiding this comment.
Some sanity questions (mainly for validation and to double check, not blocking if answered positive) - resulting API spec looks as expected. Thanks.
Wondering though that the exceptions/error response codes are not handled natively.
Also wondering a bit where are the exceptions mapped to error codes?
| details: str | dict | ||
|
|
||
|
|
||
| ValidationError |
There was a problem hiding this comment.
What is this for? A copy&paste leftover?
|
|
||
|
|
||
| @dags_router.patch("/dags/{dag_id}") | ||
| @dags_router.patch("/dags/{dag_id}", responses=create_openapi_http_exception_doc([400, 401, 403, 404])) |
There was a problem hiding this comment.
Interesting that this is not natively supported. Was it that our previous approach was not according to design of FastAPI or is this missin gin the copcepts?
| dags=[DAGResponse.model_validate(dag, from_attributes=True) for dag in dags], | ||
| total_entries=total_entries, | ||
| ) | ||
| except ValueError as e: |
There was a problem hiding this comment.
Is ValueErrorHandled by FastAPI out-of-the-box and added to api specs?
There was a problem hiding this comment.
Why is a serializer needed and where is this used/imported?
Seems to be 100% redundant to airflow.api_fastapi.openapi.exceptions.HTTPExceptionResponse - could we just import or inherit this?
