pierrejeambrun commented on code in PR #53429:
URL: https://github.com/apache/airflow/pull/53429#discussion_r2221931989
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/dags.py:
##########
@@ -191,3 +195,36 @@ def get_dags(
total_entries=total_entries,
dags=list(dag_runs_by_dag_id.values()),
)
+
+
+@dags_router.get(
+ "/{dag_id}/latest_run",
+ responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+ dependencies=[Depends(requires_access_dag(method="GET",
access_entity=DagAccessEntity.RUN))],
+)
+def get_latest_run_info(dag_id: str, session: SessionDep) ->
list[DAGRunLightResponse]:
+ """Get latest run."""
+ if dag_id == "~":
+ raise HTTPException(
+ status.HTTP_400_BAD_REQUEST,
+ "`~` was supplied as dag_id, but querying multiple dags is not
supported.",
Review Comment:
> I think the suggestion to combine get_latest_run_info and get_latest_run
is much
Yes I forgot about `get_latest_run`, since we already have that yes please
merge them and I'm fine using that.
On the other hand, regarding the public endpoint:
> get_dag_runs is slow and makes a much bigger query
The public endpiont taking 1.3 seconds to fetch a single item (limit=1) and
serialize it is seriously concerning. Something is wrong. Even if the
serializers are different and holds more field compared to the private endpoint
in this PR, accessing the DB, preloading options (which I think is missing) and
serializing 1 item should literally takes about the same time. Does your
install have any specific setup, what is actually taking all that time, that
might be worth to take a look at that for public API improvement, I would
expect a response time <200ms ? If we fix it we might don't even need that new
endpoint
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/dags.py:
##########
@@ -191,3 +195,36 @@ def get_dags(
total_entries=total_entries,
dags=list(dag_runs_by_dag_id.values()),
)
+
+
+@dags_router.get(
+ "/{dag_id}/latest_run",
+ responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+ dependencies=[Depends(requires_access_dag(method="GET",
access_entity=DagAccessEntity.RUN))],
+)
+def get_latest_run_info(dag_id: str, session: SessionDep) ->
list[DAGRunLightResponse]:
+ """Get latest run."""
+ if dag_id == "~":
+ raise HTTPException(
+ status.HTTP_400_BAD_REQUEST,
+ "`~` was supplied as dag_id, but querying multiple dags is not
supported.",
Review Comment:
> I also don't buy the maintenance argument. Like, look ta this endpoint.
Well it's still 2 new endpoints to maintain while they shouldn't exist at
all. Listing the items with limit 1 and descending order on a specific
criteria, in a decent response time (hundred of milliseconds) is a basic
feature for a LIST endpoint. Public endpoint does not need to be super
polymorphic, just implement the basic features of a restfull list endpoint.
Since we already merged `get_latest_run` that's not relevant but I'm
concerned for the performance of that public endpoint.
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/ui/dags.py:
##########
@@ -191,3 +195,36 @@ def get_dags(
total_entries=total_entries,
dags=list(dag_runs_by_dag_id.values()),
)
+
+
+@dags_router.get(
+ "/{dag_id}/latest_run",
+ responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+ dependencies=[Depends(requires_access_dag(method="GET",
access_entity=DagAccessEntity.RUN))],
+)
+def get_latest_run_info(dag_id: str, session: SessionDep) ->
list[DAGRunLightResponse]:
+ """Get latest run."""
+ if dag_id == "~":
+ raise HTTPException(
+ status.HTTP_400_BAD_REQUEST,
+ "`~` was supplied as dag_id, but querying multiple dags is not
supported.",
Review Comment:
> regarding readable_dag_runs_filter
Yes you're right we don't need to change anything.
That's because `dag_id` is in the path parameters, I got confused because
that endpoint doesn't make sense to me from a REST perspective (accessing a
single nested resource on something that looks like a LIST endpoint but is not,
since we already have that I won't argue)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
