pierrejeambrun commented on code in PR #49361:
URL: https://github.com/apache/airflow/pull/49361#discussion_r2049350564


##########
airflow-core/src/airflow/ui/src/queries/useDags.tsx:
##########
@@ -42,16 +42,15 @@ export const useDags = (
 
   const refetchInterval = useAutoRefresh({});
 
-  const { orderBy, ...runsParams } = searchParams;
   const {
     data: runsData,
     error: runsError,
     isFetching: isRunsFetching,
     isLoading: isRunsLoading,
   } = useDagsServiceRecentDagRuns(
     {
-      ...runsParams,
-      dagRunsLimit: 14,
+      ...searchParams,
+      dagRunsLimit: 1,

Review Comment:
   Actually that's a great question, because the current `recent_dag_runs` 
endpoints was supposed to be exactly that . I admit the name is bad, but it 
actually list Dags, which is a `DAGResponse` with an extra field for 
`latest_dag_runs`.
   
   So the UI should basically never use the public dag listing endpoint but 
only this one.
   
   Why is `useDags` doing some kind of weird join in the front-ent ? 
(there-fore mismatch happens), that's because I assume the private endpoint is 
missing some query parameters (search, filter, etc....).
   
   What we should do is improve that ui endpoint. (And maybe rename it to 
`ui/dags`) instead to avoid the confusion, to allow the same sorting, ordering 
searching as we do for the `api/v2/dags` one. Then we can refactor the 
`useDags` custom hook to only request that ui endpoint.



##########
airflow-core/src/airflow/ui/src/queries/useDags.tsx:
##########
@@ -42,16 +42,15 @@ export const useDags = (
 
   const refetchInterval = useAutoRefresh({});
 
-  const { orderBy, ...runsParams } = searchParams;
   const {
     data: runsData,
     error: runsError,
     isFetching: isRunsFetching,
     isLoading: isRunsLoading,
   } = useDagsServiceRecentDagRuns(
     {
-      ...runsParams,
-      dagRunsLimit: 14,
+      ...searchParams,
+      dagRunsLimit: 1,

Review Comment:
   >At this point, it might make more sense to create a separate DAGs list 
endpoint specifically for the UI, with recent DAG runs already pre-joined, 
separating concerns from the public API.
   
   
   
   Actually that's a great question, because the current `recent_dag_runs` 
endpoints was supposed to be exactly that . I admit the name is bad, but it 
actually list Dags, which is a `DAGResponse` with an extra field for 
`latest_dag_runs`.
   
   So the UI should basically never use the public dag listing endpoint but 
only this one.
   
   Why is `useDags` doing some kind of weird join in the front-ent ? 
(there-fore mismatch happens), that's because I assume the private endpoint is 
missing some query parameters (search, filter, etc....).
   
   What we should do is improve that ui endpoint. (And maybe rename it to 
`ui/dags`) instead to avoid the confusion, to allow the same sorting, ordering 
searching as we do for the `api/v2/dags` one. Then we can refactor the 
`useDags` custom hook to only request that ui endpoint.



-- 
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]

Reply via email to