AIP-84: Add UI batch recent dag runs endpoint

[jason810496]

closes: #42933
Same filter with public get_dags endpoint, but remove order_by params.

[pierrejeambrun]

Nice

[jason810496]

Refactor:

Used DAGRunResponse, DAGCollectionResponse, and DAGResponse from the public serializers for the UI batch of recent runs. However, the subclasses that inherit from the public serializers should also inherit from OptionalModel, as most of the fields should be optional.

[pierrejeambrun]

I am not sure I understand the OptionalModel. Why do those DAGRuns and DAGs omit fields in the private response compare to the public one ?

I would assume that all of the public fields are also defined and here in the private endpoint therefore removing the need for that OptionalModel class. Did you encounter a specific issue ?

[jason810496]

I think for this UI, the endpoint doesn't need to respond with so much information (compared with /last_dagruns in views.py). In /last_dagruns, it only needs dag_id, start_date, end_date, state, execution_date, data_interval_start, and data_interval_end.

Instead of marking the fields that aren't needed in this endpoint as optional in DAGRunResponse, I think inheriting from OptionalModel to mark the entire RecentDAGRunResponse would be clearer.

[pierrejeambrun]

I think we can just return the whole information and don't bother filtering things out. For the front-end it is easier because you only manipulate 1 type for the returned DAG, DagRun, etc... You do not have different representation depending on the endpoint (private/public), which makes things easier to work with (fill/cast etc..).

Note: Most likely not needed here, but if you want to return a subpart of a model, the pydantic documentation explains it well, you can jus use the response_model, with a minimal model having only the fields that you need. https://fastapi.tiangolo.com/tutorial/response-model/#response_model-parameter. We would most likely not use a custom all optional OptionalModel for that.

[jason810496]

Resolved:

Made this UI endpoint more consistent with the public endpoint by removing OptionalModel and using DAGRunResponse directly from the public endpoint instead.

[pierrejeambrun]

@jason810496 Nice, I just added a small commit on top to make a few adjustment, let me know what you think, I believe we can merge then.

[pierrejeambrun]

NIce

[jason810496]

Hi @pierrejeambrun, the changes look good to me, but I have another question:

When should I utilize a parameter class (the classes declared in common/parameters.py with to_orm and depends methods) instead of adding query parameters directly (like dag_runs_limit: int = 10 in this case)?

For example, in the Event Log endpoints, the query should leverage paginated_select, but the paginated_select function depends on a parameter class rather than query parameters. In this case, should I declare all filter classes in common/parameters.py, or would it be better to add a FilterParam with dynamic_depends, similar to SortParam, that could be used across all endpoints?

[pierrejeambrun]

When should I utilize a parameter class (the classes declared in common/parameters.py with to_orm and depends methods) instead of adding query parameters directly (like dag_runs_limit: int = 10 in this case)?
For example, in the Event Log endpoints, the query should leverage paginated_select, but the paginated_select function depends on a parameter class rather than query parameters. In this case, should I declare all filter classes in common/parameters.py, or would it be better to add a FilterParam with dynamic_depends, similar to SortParam, that could be used across all endpoints?

The idea is if the parameter (query or path) is meant to be re-used in other endpoints and implement a common db operation (filtering, sorting, matching, anything), then it should go into parameters with our common parameters.

If like here it is a very specific one, in terms of parameters (a secondary limit index operating on nested ressources) in terms of naming "dag_runs_limit", and in terms of implementation (.where(recent_runs_subquery.c.rank <= dag_runs_limit) which is really specific to the subquery), then it's ok to leave it in the router and not bother creating a reusable 'parameters' out of if.