pierrejeambrun commented on code in PR #50957:
URL: https://github.com/apache/airflow/pull/50957#discussion_r2120350829
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/dag_run.py:
##########
@@ -68,6 +69,7 @@ class DAGRunResponse(BaseModel):
end_date: datetime | None
data_interval_start: datetime | None
data_interval_end: datetime | None
+ deadlines: list[DeadlineResponse] | None
Review Comment:
Maybe I misunderstood something. As I understand it, there's no record in
the db at all for deadlines unless the run is in progress?
I assume we can always improve later and add that if people request it.
(soft delete etc...)
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_dag_run.py:
##########
@@ -166,6 +180,13 @@ def get_dag_versions_dict(dag_versions: list[DagVersion])
-> list[dict]:
]
+def convert_instrumented_list_to_dict(lst):
+ result = [obj.__dict__ for obj in lst]
+ for obj in result:
+ obj.pop("_sa_instance_state", None)
+ return result
+
+
Review Comment:
Is that needed ?
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/dag_run.py:
##########
@@ -68,6 +69,7 @@ class DAGRunResponse(BaseModel):
end_date: datetime | None
data_interval_start: datetime | None
data_interval_end: datetime | None
+ deadlines: list[DeadlineResponse] | None
Review Comment:
`deadlines: list[DeadlineResponse]` should be enough, the relationship
should return an empty list when there is no related items
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/dag_run.py:
##########
@@ -68,6 +69,7 @@ class DAGRunResponse(BaseModel):
end_date: datetime | None
data_interval_start: datetime | None
data_interval_end: datetime | None
+ deadlines: list[DeadlineResponse] | None
Review Comment:
From the API point of view, we can nest the `deadline` API inside the
`dagrun` one. Because deadline can be considered a sub-resource of the dagrun,
they only exist for their corresponding dagrun, and especially when the dagrun
is in a specific state, otherwise they are removed. (But that's debatable,
since all the API resource are for now flat at root level, we could do the same
here).
--
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]
