jedcunningham commented on code in PR #40441:
URL: https://github.com/apache/airflow/pull/40441#discussion_r1672500761
##########
airflow/api_connexion/endpoints/task_instance_endpoint.py:
##########
@@ -754,3 +754,79 @@ def get_mapped_task_instance_dependencies(
return get_task_instance_dependencies(
dag_id=dag_id, dag_run_id=dag_run_id, task_id=task_id,
map_index=map_index
)
+
+
[email protected]_access_dag("GET", DagAccessEntity.TASK_INSTANCE)
+@provide_session
+def get_task_instance_try_details(
+ *,
+ dag_id: str,
+ dag_run_id: str,
+ task_id: str,
+ task_try_number: int,
+ map_index: int = -1,
+ session: Session = NEW_SESSION,
+) -> APIResponse:
+ """Get details of a task instance try."""
+ from airflow.models.taskinstancehistory import TaskInstanceHistory
+
+ def _query(orm_object):
+ query = select(orm_object).where(
+ orm_object.dag_id == dag_id,
+ orm_object.run_id == dag_run_id,
+ orm_object.task_id == task_id,
+ orm_object.try_number == task_try_number,
+ orm_object.map_index == map_index,
+ )
+
+ query = (
+ query.join(orm_object.dag_run)
+ .outerjoin(
+ SlaMiss,
+ and_(
+ SlaMiss.dag_id == orm_object.dag_id,
+ SlaMiss.execution_date == DR.execution_date,
+ SlaMiss.task_id == orm_object.task_id,
+ ),
+ )
+ .add_columns(SlaMiss)
+ .options(joinedload(orm_object.rendered_task_instance_fields))
+ )
+ try:
+ result = session.execute(query).one_or_none()
+ except MultipleResultsFound:
+ raise NotFound(
+ "Task instance not found",
+ detail="Task instance is mapped, add the map_index value to
the URL",
+ )
+ return result
+
+ result = _query(TI)
+
+ if not result:
+ result = _query(TaskInstanceHistory)
Review Comment:
```suggestion
result = _query(TI) or _query(TaskInstanceHistory)
```
##########
tests/api_connexion/endpoints/test_task_instance_endpoint.py:
##########
Review Comment:
It might be nice to have an explicit test that shows the `query(TI or
TIHistory)` behavior works. I _think_ we cover it since we have tests here that
end up hitting both tables, but that'd be easy to accidentally refactor out.
Having it as an explicit test would make it easier to ensure its covered and
stayed covered long term.
##########
airflow/api_connexion/openapi/v1.yaml:
##########
@@ -1720,6 +1720,65 @@ paths:
"404":
$ref: "#/components/responses/NotFound"
+
/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/tries/{task_try_number}:
+ get:
+ summary: get taskinstance try
+ description: |
+ Get details of a task instance try.
+
+ *New in version 2.10.0*
+ x-openapi-router-controller:
airflow.api_connexion.endpoints.task_instance_endpoint
+ operationId: get_task_instance_try_details
+ tags: [TaskInstance]
+ parameters:
+ - $ref: "#/components/parameters/DAGID"
+ - $ref: "#/components/parameters/DAGRunID"
+ - $ref: "#/components/parameters/TaskID"
+ - $ref: "#/components/parameters/TaskTryNumber"
+ responses:
+ "200":
+ description: Success.
+ content:
+ application/json:
+ schema:
+ $ref: "#/components/schemas/TaskInstance"
+ "401":
+ $ref: "#/components/responses/Unauthenticated"
+ "403":
+ $ref: "#/components/responses/PermissionDenied"
+ "404":
+ $ref: "#/components/responses/NotFound"
+
+
/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/tries/{task_try_number}/{map_index}:
Review Comment:
Probably makes sense to put map_index farther up the path? Maybe
`/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/{map_index}/tries/{task_try_number}`?
That matches the other mapped ti endpoint better.
--
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]
