You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "kandharvishnu (via GitHub)" <gi...@apache.org> on 2024/11/05 07:17:47 UTC
[PR] AIP-84 Get Task Instance Try Details [airflow]
kandharvishnu opened a new pull request, #43675:
URL: https://github.com/apache/airflow/pull/43675
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
related to: https://github.com/apache/airflow/issues/42370
<!-- Please keep an empty line above the dashes. -->
---
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "kandharvishnu (via GitHub)" <gi...@apache.org>.
kandharvishnu commented on code in PR #43675:
URL: https://github.com/apache/airflow/pull/43675#discussion_r1828905533
##########
tests/api_fastapi/core_api/routes/public/test_task_instances.py:
##########
@@ -143,20 +143,6 @@ def create_task_instances(
session.commit()
return tis
- session.commit()
Review Comment:
This code appears to be repetitive, so I’m removing it.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "kandharvishnu (via GitHub)" <gi...@apache.org>.
kandharvishnu commented on code in PR #43675:
URL: https://github.com/apache/airflow/pull/43675#discussion_r1828905533
##########
tests/api_fastapi/core_api/routes/public/test_task_instances.py:
##########
@@ -143,20 +143,6 @@ def create_task_instances(
session.commit()
return tis
- session.commit()
Review Comment:
This code appears to be repetitive, so I’m removing it.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #43675:
URL: https://github.com/apache/airflow/pull/43675#issuecomment-2479433434
Need rebasing as well.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #43675:
URL: https://github.com/apache/airflow/pull/43675#discussion_r1846174979
##########
tests/api_fastapi/core_api/routes/public/test_task_instances.py:
##########
@@ -1127,3 +1128,174 @@ def test_should_respond_dependencies_mapped(self, test_client, session):
"print_the_context/0/dependencies",
)
assert response.status_code == 200, response.text
+
+
+class TestGetTaskInstanceTry(TestTaskInstanceEndpoint):
+ def test_should_respond_200(self, test_client, session):
+ self.create_task_instances(session, task_instances=[{"state": State.SUCCESS}], with_ti_history=True)
+ response = test_client.get(
+ "/public/dags/example_python_operator/dagRuns/TEST_DAG_RUN_ID/taskInstances/print_the_context/tries/1"
+ )
+ assert response.status_code == 200
+ assert response.json() == {
+ "dag_id": "example_python_operator",
+ "duration": 10000.0,
+ "end_date": "2020-01-03T00:00:00Z",
+ "executor": None,
+ "executor_config": "{}",
+ "hostname": "",
+ "map_index": -1,
+ "max_tries": 0,
+ "operator": "PythonOperator",
+ "pid": 100,
+ "pool": "default_pool",
+ "pool_slots": 1,
+ "priority_weight": 9,
+ "queue": "default_queue",
+ "queued_when": None,
+ "start_date": "2020-01-02T00:00:00Z",
+ "state": "success",
+ "task_id": "print_the_context",
+ "task_display_name": "print_the_context",
+ "try_number": 1,
+ "unixname": getuser(),
+ "dag_run_id": "TEST_DAG_RUN_ID",
+ }
+
Review Comment:
Oh, that's a different endpoint for mapped task, got it.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #43675:
URL: https://github.com/apache/airflow/pull/43675#discussion_r1831091753
##########
tests/api_fastapi/core_api/routes/public/test_task_instances.py:
##########
@@ -465,3 +451,55 @@ def test_should_respond_404_wrong_map_index(self, test_client, session):
assert response.json() == {
"detail": "The Mapped Task Instance with dag_id: `example_python_operator`, run_id: `TEST_DAG_RUN_ID`, task_id: `print_the_context`, and map_index: `10` was not found"
}
+
+
+class TestGetTaskInstanceTryDetails(TestTaskInstanceEndpoint):
+ def test_should_respond_200_get_task_instance_try_details(self, test_client, session):
+ self.create_task_instances(session, task_instances=[{"try_number": 1}])
+ response = test_client.get(
+ "/public/dags/example_python_operator/dagRuns/TEST_DAG_RUN_ID/taskInstances/print_the_context/tries/1"
+ )
+ assert response.status_code == 200
+
+ assert response.json() == {
+ "dag_id": "example_python_operator",
+ "duration": 10000.0,
+ "end_date": "2020-01-03T00:00:00Z",
+ "logical_date": "2020-01-01T00:00:00Z",
+ "executor": None,
+ "executor_config": "{}",
+ "hostname": "",
+ "id": mock.ANY,
+ "map_index": -1,
+ "max_tries": 0,
+ "note": "placeholder-note",
+ "operator": "PythonOperator",
+ "pid": 100,
+ "pool": "default_pool",
+ "pool_slots": 1,
+ "priority_weight": 9,
+ "queue": "default_queue",
+ "queued_when": None,
+ "start_date": "2020-01-02T00:00:00Z",
+ "state": "running",
+ "task_id": "print_the_context",
+ "task_display_name": "print_the_context",
+ "try_number": 1,
+ "unixname": getuser(),
+ "dag_run_id": "TEST_DAG_RUN_ID",
+ "rendered_fields": {},
+ "rendered_map_index": None,
+ "trigger": None,
+ "triggerer_job": None,
+ }
+
+ def test_should_respond_404_wrong_task_instance_try_details(self, test_client, session):
Review Comment:
Or refer to legecy tests [here](https://github.com/apache/airflow/blob/c559b208949832d81c0a823ace04d3e172f0a842/tests/api_connexion/endpoints/test_task_instance_endpoint.py#L2609)
##########
tests/api_fastapi/core_api/routes/public/test_task_instances.py:
##########
@@ -465,3 +451,55 @@ def test_should_respond_404_wrong_map_index(self, test_client, session):
assert response.json() == {
"detail": "The Mapped Task Instance with dag_id: `example_python_operator`, run_id: `TEST_DAG_RUN_ID`, task_id: `print_the_context`, and map_index: `10` was not found"
}
+
+
+class TestGetTaskInstanceTryDetails(TestTaskInstanceEndpoint):
+ def test_should_respond_200_get_task_instance_try_details(self, test_client, session):
+ self.create_task_instances(session, task_instances=[{"try_number": 1}])
+ response = test_client.get(
+ "/public/dags/example_python_operator/dagRuns/TEST_DAG_RUN_ID/taskInstances/print_the_context/tries/1"
+ )
+ assert response.status_code == 200
+
+ assert response.json() == {
+ "dag_id": "example_python_operator",
+ "duration": 10000.0,
+ "end_date": "2020-01-03T00:00:00Z",
+ "logical_date": "2020-01-01T00:00:00Z",
+ "executor": None,
+ "executor_config": "{}",
+ "hostname": "",
+ "id": mock.ANY,
+ "map_index": -1,
+ "max_tries": 0,
+ "note": "placeholder-note",
+ "operator": "PythonOperator",
+ "pid": 100,
+ "pool": "default_pool",
+ "pool_slots": 1,
+ "priority_weight": 9,
+ "queue": "default_queue",
+ "queued_when": None,
+ "start_date": "2020-01-02T00:00:00Z",
+ "state": "running",
+ "task_id": "print_the_context",
+ "task_display_name": "print_the_context",
+ "try_number": 1,
+ "unixname": getuser(),
+ "dag_run_id": "TEST_DAG_RUN_ID",
+ "rendered_fields": {},
+ "rendered_map_index": None,
+ "trigger": None,
+ "triggerer_job": None,
+ }
+
+ def test_should_respond_404_wrong_task_instance_try_details(self, test_client, session):
Review Comment:
Or refer to legacy tests [here](https://github.com/apache/airflow/blob/c559b208949832d81c0a823ace04d3e172f0a842/tests/api_connexion/endpoints/test_task_instance_endpoint.py#L2609)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #43675:
URL: https://github.com/apache/airflow/pull/43675#issuecomment-2482418951
Needs rebasing. We can merge after the small adjustments are done.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #43675:
URL: https://github.com/apache/airflow/pull/43675#discussion_r1843322349
##########
airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -335,3 +337,40 @@ def get_task_instances(
],
total_entries=total_entries,
)
+
+
+@task_instances_router.get(
+ "/{task_id}/tries/{task_try_number}",
+ responses=create_openapi_http_exception_doc(
+ [status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN, status.HTTP_404_NOT_FOUND]
Review Comment:
we don't need to add 401, 403 separately. They are now automatically added
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #43675:
URL: https://github.com/apache/airflow/pull/43675#discussion_r1843264816
##########
airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -335,3 +337,37 @@ def get_task_instances(
],
total_entries=total_entries,
)
+
+
+@task_instances_router.get(
+ "/{task_id}/tries/{task_try_number}", responses=create_openapi_http_exception_doc([401, 403, 404])
Review Comment:
401 and 403 are now added to the base router. Refer to https://github.com/apache/airflow/pull/43932
```suggestion
"/{task_id}/tries/{task_try_number}", responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND])
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #43675:
URL: https://github.com/apache/airflow/pull/43675#discussion_r1843625993
##########
airflow/api_fastapi/core_api/datamodels/task_instances.py:
##########
@@ -83,3 +83,39 @@ class TaskDependencyCollectionResponse(BaseModel):
"""Task scheduling dependencies collection serializer for responses."""
dependencies: list[TaskDependencyResponse]
+
+
+class TaskInstanceHistoryResponse(BaseModel):
+ """TaskInstance serializer for responses."""
Review Comment:
```suggestion
"""TaskInstanceHistory serializer for responses."""
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun merged PR #43675:
URL: https://github.com/apache/airflow/pull/43675
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "kandharvishnu (via GitHub)" <gi...@apache.org>.
kandharvishnu commented on code in PR #43675:
URL: https://github.com/apache/airflow/pull/43675#discussion_r1843327639
##########
airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -335,3 +337,40 @@ def get_task_instances(
],
total_entries=total_entries,
)
+
+
+@task_instances_router.get(
+ "/{task_id}/tries/{task_try_number}",
+ responses=create_openapi_http_exception_doc(
+ [status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN, status.HTTP_404_NOT_FOUND]
Review Comment:
oh, got it
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #43675:
URL: https://github.com/apache/airflow/pull/43675#discussion_r1844127259
##########
airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -335,3 +337,40 @@ def get_task_instances(
],
total_entries=total_entries,
)
+
+
+@task_instances_router.get(
+ "/{task_id}/tries/{task_try_number}",
+ responses=create_openapi_http_exception_doc(
+ [status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN, status.HTTP_404_NOT_FOUND]
+ ),
+)
+async def get_task_instance_try_details(
+ dag_id: str,
+ dag_run_id: str,
+ task_id: str,
+ task_try_number: int,
+ session: Annotated[Session, Depends(get_session)],
+ map_index: int = -1,
+) -> TaskInstanceHistoryResponse:
+ """Get task instance details by try number."""
+
+ def _query(TI):
+ query = select(TI).where(
+ TI.dag_id == dag_id,
+ TI.run_id == dag_run_id,
+ TI.task_id == task_id,
+ TI.try_number == task_try_number,
+ TI.map_index == map_index,
+ )
+
+ task_instance = session.scalar(query)
+ return task_instance
+
+ result = _query(TI) or _query(TIH)
+ if result is None:
+ raise HTTPException(
+ status.HTTP_404_NOT_FOUND,
+ f"The Task Instance with dag_id: `{dag_id}`, run_id: `{dag_run_id}`, task_id: `{task_id}`, try_number: `{task_try_number}` and map_index: `{map_index}` was not found",
+ )
+ return TaskInstanceHistoryResponse.model_validate(result, from_attributes=True)
Review Comment:
As we saw that there was some small differences bestween `TaskInstanceHistoryResponse` and `TaskInstanceResponse`, I would say that `TaskInstanceHistoryResponse` is the most appropriate.
##########
airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -335,3 +337,40 @@ def get_task_instances(
],
total_entries=total_entries,
)
+
+
+@task_instances_router.get(
+ "/{task_id}/tries/{task_try_number}",
+ responses=create_openapi_http_exception_doc(
+ [status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN, status.HTTP_404_NOT_FOUND]
+ ),
+)
+async def get_task_instance_try_details(
+ dag_id: str,
+ dag_run_id: str,
+ task_id: str,
+ task_try_number: int,
+ session: Annotated[Session, Depends(get_session)],
+ map_index: int = -1,
+) -> TaskInstanceHistoryResponse:
+ """Get task instance details by try number."""
+
+ def _query(TI):
+ query = select(TI).where(
+ TI.dag_id == dag_id,
+ TI.run_id == dag_run_id,
+ TI.task_id == task_id,
+ TI.try_number == task_try_number,
+ TI.map_index == map_index,
+ )
+
+ task_instance = session.scalar(query)
+ return task_instance
+
+ result = _query(TI) or _query(TIH)
+ if result is None:
+ raise HTTPException(
+ status.HTTP_404_NOT_FOUND,
+ f"The Task Instance with dag_id: `{dag_id}`, run_id: `{dag_run_id}`, task_id: `{task_id}`, try_number: `{task_try_number}` and map_index: `{map_index}` was not found",
+ )
+ return TaskInstanceHistoryResponse.model_validate(result, from_attributes=True)
Review Comment:
As we saw that there was some small differences bestween `TaskInstanceHistoryResponse` and `TaskInstanceResponse`, I would say that `TaskInstanceHistoryResponse` is the most appropriate. (Those are not fully exchangeable)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #43675:
URL: https://github.com/apache/airflow/pull/43675#discussion_r1831086023
##########
tests/api_fastapi/core_api/routes/public/test_task_instances.py:
##########
@@ -465,3 +451,55 @@ def test_should_respond_404_wrong_map_index(self, test_client, session):
assert response.json() == {
"detail": "The Mapped Task Instance with dag_id: `example_python_operator`, run_id: `TEST_DAG_RUN_ID`, task_id: `print_the_context`, and map_index: `10` was not found"
}
+
+
+class TestGetTaskInstanceTryDetails(TestTaskInstanceEndpoint):
+ def test_should_respond_200_get_task_instance_try_details(self, test_client, session):
+ self.create_task_instances(session, task_instances=[{"try_number": 1}])
+ response = test_client.get(
+ "/public/dags/example_python_operator/dagRuns/TEST_DAG_RUN_ID/taskInstances/print_the_context/tries/1"
+ )
+ assert response.status_code == 200
+
+ assert response.json() == {
+ "dag_id": "example_python_operator",
+ "duration": 10000.0,
+ "end_date": "2020-01-03T00:00:00Z",
+ "logical_date": "2020-01-01T00:00:00Z",
+ "executor": None,
+ "executor_config": "{}",
+ "hostname": "",
+ "id": mock.ANY,
+ "map_index": -1,
+ "max_tries": 0,
+ "note": "placeholder-note",
+ "operator": "PythonOperator",
+ "pid": 100,
+ "pool": "default_pool",
+ "pool_slots": 1,
+ "priority_weight": 9,
+ "queue": "default_queue",
+ "queued_when": None,
+ "start_date": "2020-01-02T00:00:00Z",
+ "state": "running",
+ "task_id": "print_the_context",
+ "task_display_name": "print_the_context",
+ "try_number": 1,
+ "unixname": getuser(),
+ "dag_run_id": "TEST_DAG_RUN_ID",
+ "rendered_fields": {},
+ "rendered_map_index": None,
+ "trigger": None,
+ "triggerer_job": None,
+ }
+
+ def test_should_respond_404_wrong_task_instance_try_details(self, test_client, session):
Review Comment:
Can you also add a test for "Multiple Task Instances" found error?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "kandharvishnu (via GitHub)" <gi...@apache.org>.
kandharvishnu commented on code in PR #43675:
URL: https://github.com/apache/airflow/pull/43675#discussion_r1843305340
##########
airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -335,3 +337,40 @@ def get_task_instances(
],
total_entries=total_entries,
)
+
+
+@task_instances_router.get(
+ "/{task_id}/tries/{task_try_number}",
+ responses=create_openapi_http_exception_doc(
+ [status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN, status.HTTP_404_NOT_FOUND]
+ ),
+)
+async def get_task_instance_try_details(
+ dag_id: str,
+ dag_run_id: str,
+ task_id: str,
+ task_try_number: int,
+ session: Annotated[Session, Depends(get_session)],
+ map_index: int = -1,
+) -> TaskInstanceHistoryResponse:
+ """Get task instance details by try number."""
+
+ def _query(TI):
+ query = select(TI).where(
+ TI.dag_id == dag_id,
+ TI.run_id == dag_run_id,
+ TI.task_id == task_id,
+ TI.try_number == task_try_number,
+ TI.map_index == map_index,
+ )
+
+ task_instance = session.scalar(query)
+ return task_instance
+
+ result = _query(TI) or _query(TIH)
+ if result is None:
+ raise HTTPException(
+ status.HTTP_404_NOT_FOUND,
+ f"The Task Instance with dag_id: `{dag_id}`, run_id: `{dag_run_id}`, task_id: `{task_id}`, try_number: `{task_try_number}` and map_index: `{map_index}` was not found",
+ )
+ return TaskInstanceHistoryResponse.model_validate(result, from_attributes=True)
Review Comment:
@pierrejeambrun @ephraimbuddy shall we return `TaskInstanceHistoryResponse` or change it to `TaskInstanceResponse`
[related PR](https://github.com/apache/airflow/pull/43830)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "kandharvishnu (via GitHub)" <gi...@apache.org>.
kandharvishnu commented on code in PR #43675:
URL: https://github.com/apache/airflow/pull/43675#discussion_r1844189214
##########
tests/api_fastapi/core_api/routes/public/test_task_instances.py:
##########
@@ -1127,3 +1128,174 @@ def test_should_respond_dependencies_mapped(self, test_client, session):
"print_the_context/0/dependencies",
)
assert response.status_code == 200, response.text
+
+
+class TestGetTaskInstanceTry(TestTaskInstanceEndpoint):
+ def test_should_respond_200(self, test_client, session):
+ self.create_task_instances(session, task_instances=[{"state": State.SUCCESS}], with_ti_history=True)
+ response = test_client.get(
+ "/public/dags/example_python_operator/dagRuns/TEST_DAG_RUN_ID/taskInstances/print_the_context/tries/1"
+ )
+ assert response.status_code == 200
+ assert response.json() == {
+ "dag_id": "example_python_operator",
+ "duration": 10000.0,
+ "end_date": "2020-01-03T00:00:00Z",
+ "executor": None,
+ "executor_config": "{}",
+ "hostname": "",
+ "map_index": -1,
+ "max_tries": 0,
+ "operator": "PythonOperator",
+ "pid": 100,
+ "pool": "default_pool",
+ "pool_slots": 1,
+ "priority_weight": 9,
+ "queue": "default_queue",
+ "queued_when": None,
+ "start_date": "2020-01-02T00:00:00Z",
+ "state": "success",
+ "task_id": "print_the_context",
+ "task_display_name": "print_the_context",
+ "try_number": 1,
+ "unixname": getuser(),
+ "dag_run_id": "TEST_DAG_RUN_ID",
+ }
+
Review Comment:
This method needs to be added after the implementation of `get_mapped_task_instance_try_details`
https://github.com/apache/airflow/blob/fd0a0d2eb2f24452e67f25a3de1eb6e27a901203/tests/api_connexion/endpoints/test_task_instance_endpoint.py#L2708
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #43675:
URL: https://github.com/apache/airflow/pull/43675#discussion_r1831086023
##########
tests/api_fastapi/core_api/routes/public/test_task_instances.py:
##########
@@ -465,3 +451,55 @@ def test_should_respond_404_wrong_map_index(self, test_client, session):
assert response.json() == {
"detail": "The Mapped Task Instance with dag_id: `example_python_operator`, run_id: `TEST_DAG_RUN_ID`, task_id: `print_the_context`, and map_index: `10` was not found"
}
+
+
+class TestGetTaskInstanceTryDetails(TestTaskInstanceEndpoint):
+ def test_should_respond_200_get_task_instance_try_details(self, test_client, session):
+ self.create_task_instances(session, task_instances=[{"try_number": 1}])
+ response = test_client.get(
+ "/public/dags/example_python_operator/dagRuns/TEST_DAG_RUN_ID/taskInstances/print_the_context/tries/1"
+ )
+ assert response.status_code == 200
+
+ assert response.json() == {
+ "dag_id": "example_python_operator",
+ "duration": 10000.0,
+ "end_date": "2020-01-03T00:00:00Z",
+ "logical_date": "2020-01-01T00:00:00Z",
+ "executor": None,
+ "executor_config": "{}",
+ "hostname": "",
+ "id": mock.ANY,
+ "map_index": -1,
+ "max_tries": 0,
+ "note": "placeholder-note",
+ "operator": "PythonOperator",
+ "pid": 100,
+ "pool": "default_pool",
+ "pool_slots": 1,
+ "priority_weight": 9,
+ "queue": "default_queue",
+ "queued_when": None,
+ "start_date": "2020-01-02T00:00:00Z",
+ "state": "running",
+ "task_id": "print_the_context",
+ "task_display_name": "print_the_context",
+ "try_number": 1,
+ "unixname": getuser(),
+ "dag_run_id": "TEST_DAG_RUN_ID",
+ "rendered_fields": {},
+ "rendered_map_index": None,
+ "trigger": None,
+ "triggerer_job": None,
+ }
+
+ def test_should_respond_404_wrong_task_instance_try_details(self, test_client, session):
Review Comment:
Can you also add a test for "Multiple Task Instances" found error? If this case is not even possible to occur, maybe its not required?
@pierrejeambrun , wdyt?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #43675:
URL: https://github.com/apache/airflow/pull/43675#discussion_r1844182536
##########
airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -331,3 +333,38 @@ def get_task_instances(
],
total_entries=total_entries,
)
+
+
+@task_instances_router.get(
+ "/{task_id}/tries/{task_try_number}",
+ responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+)
+def get_task_instance_try_details(
+ dag_id: str,
+ dag_run_id: str,
+ task_id: str,
+ task_try_number: int,
+ session: Annotated[Session, Depends(get_session)],
+ map_index: int = -1,
+) -> TaskInstanceHistoryResponse:
+ """Get task instance details by try number."""
+
+ def _query(TI):
Review Comment:
Also you can add typing (param and return type) Select
##########
airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -331,3 +333,38 @@ def get_task_instances(
],
total_entries=total_entries,
)
+
+
+@task_instances_router.get(
+ "/{task_id}/tries/{task_try_number}",
+ responses=create_openapi_http_exception_doc([status.HTTP_404_NOT_FOUND]),
+)
+def get_task_instance_try_details(
+ dag_id: str,
+ dag_run_id: str,
+ task_id: str,
+ task_try_number: int,
+ session: Annotated[Session, Depends(get_session)],
+ map_index: int = -1,
+) -> TaskInstanceHistoryResponse:
+ """Get task instance details by try number."""
+
+ def _query(TI):
Review Comment:
```suggestion
def _query(TI):
```
```suggestion
def _query(orm_object: Base):
```
`orm_object` is more explicit.
##########
airflow/api_fastapi/core_api/datamodels/task_instances.py:
##########
@@ -83,3 +83,39 @@ class TaskDependencyCollectionResponse(BaseModel):
"""Task scheduling dependencies collection serializer for responses."""
dependencies: list[TaskDependencyResponse]
+
+
+class TaskInstanceHistoryResponse(BaseModel):
+ """TaskInstanceHistory serializer for responses."""
+
+ model_config = ConfigDict(populate_by_name=True)
+
+ task_id: str
+ dag_id: str
+ run_id: str = Field(alias="dag_run_id")
+ map_index: int
+ start_date: datetime | None
+ end_date: datetime | None
+ duration: float | None
+ state: TaskInstanceState | None
+ try_number: int
+ max_tries: int
+ task_display_name: str
+ hostname: str | None
+ unixname: str | None
+ pool: str
+ pool_slots: int
+ queue: str | None
+ priority_weight: int | None
+ operator: str | None
+ queued_dttm: datetime | None = Field(alias="queued_when")
+ pid: int | None
+ executor: str | None
+ executor_config: Annotated[str, BeforeValidator(str)]
+
+
+class TaskInstanceHistoryCollectionResponse(BaseModel):
+ """Task Instance Collection serializer for responses."""
Review Comment:
docstring need update.
##########
tests/api_fastapi/core_api/routes/public/test_task_instances.py:
##########
@@ -1127,3 +1128,174 @@ def test_should_respond_dependencies_mapped(self, test_client, session):
"print_the_context/0/dependencies",
)
assert response.status_code == 200, response.text
+
+
+class TestGetTaskInstanceTry(TestTaskInstanceEndpoint):
+ def test_should_respond_200(self, test_client, session):
+ self.create_task_instances(session, task_instances=[{"state": State.SUCCESS}], with_ti_history=True)
+ response = test_client.get(
+ "/public/dags/example_python_operator/dagRuns/TEST_DAG_RUN_ID/taskInstances/print_the_context/tries/1"
+ )
+ assert response.status_code == 200
+ assert response.json() == {
+ "dag_id": "example_python_operator",
+ "duration": 10000.0,
+ "end_date": "2020-01-03T00:00:00Z",
+ "executor": None,
+ "executor_config": "{}",
+ "hostname": "",
+ "map_index": -1,
+ "max_tries": 0,
+ "operator": "PythonOperator",
+ "pid": 100,
+ "pool": "default_pool",
+ "pool_slots": 1,
+ "priority_weight": 9,
+ "queue": "default_queue",
+ "queued_when": None,
+ "start_date": "2020-01-02T00:00:00Z",
+ "state": "success",
+ "task_id": "print_the_context",
+ "task_display_name": "print_the_context",
+ "try_number": 1,
+ "unixname": getuser(),
+ "dag_run_id": "TEST_DAG_RUN_ID",
+ }
+
Review Comment:
Any reason why `test_should_respond_200_with_mapped_task_at_different_try_numbers` was not moved over ?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on PR #43675:
URL: https://github.com/apache/airflow/pull/43675#issuecomment-2485705791
Nice! On to the next one @kandharvishnu
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AIP-84 Get Task Instance Try Details [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #43675:
URL: https://github.com/apache/airflow/pull/43675#discussion_r1843321747
##########
airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -335,3 +337,40 @@ def get_task_instances(
],
total_entries=total_entries,
)
+
+
+@task_instances_router.get(
+ "/{task_id}/tries/{task_try_number}",
+ responses=create_openapi_http_exception_doc(
+ [status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN, status.HTTP_404_NOT_FOUND]
Review Comment:
```suggestion
[status.HTTP_404_NOT_FOUND]
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org