Skip to content

Conversation

@amoghrajesh
Copy link
Contributor

@amoghrajesh amoghrajesh commented Dec 17, 2024

related: #44414

This is an addition to #44954 and is dependent on it.
Only last commit is relevant cb493ee

We do not have to handle the case of external task state updates specifically in task_runner because public UI APIs will directly do that job - core APIs. So there is no scenario when one of these exceptions are thrown due to this:

except ServerResponseError as e:
if e.response.status_code in {HTTPStatus.NOT_FOUND, HTTPStatus.CONFLICT}:
log.error(
"Server indicated the task shouldn't be running anymore",
detail=e.detail,
status_code=e.response.status_code,
)
self.kill(signal.SIGTERM, force=True)

We check with the database for the state with the heartbeat and if ever there is a different state, other than running:

if previous_state != State.RUNNING:
raise HTTPException(
status_code=status.HTTP_409_CONFLICT,
detail={
"reason": "not_running",
"message": "TI is no longer in the running state and task should terminate",
"current_state": previous_state,
},
)
, we fail with a 409.

But, if this state is ever hit, we should just mark the TI as failed. There are UTs https://github.com/apache/airflow/compare/main...astronomer:airflow:AIP72-taskterminated-or-afexception?expand=1#diff-413c3c59636a3c7b41b8bb822827d18a959778d0b6331532e0db175c829dbfd2R342-R397 which handle this portion to check if it fails or not.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@amoghrajesh amoghrajesh added area:task-execution-interface-aip72 AIP-72: Task Execution Interface (TEI) aka Task SDK area:task-sdk labels Dec 17, 2024
@amoghrajesh amoghrajesh changed the title AIP-72: Handling failed TI state for AirflowException & AirflowTaskTerminated AIP-72: Handling failed TI state for AirflowException & AirflowTaskTerminated Dec 17, 2024
@amoghrajesh amoghrajesh changed the title AIP-72: Handling failed TI state for AirflowException & AirflowTaskTerminated AIP-72: Handling failed TI state for AirflowTaskTerminated Dec 17, 2024
@amoghrajesh amoghrajesh requested review from ashb and kaxil December 17, 2024 16:19
@amoghrajesh amoghrajesh self-assigned this Dec 18, 2024
@amoghrajesh amoghrajesh merged commit 65b110d into apache:main Dec 19, 2024
48 checks passed
@amoghrajesh amoghrajesh deleted the AIP72-taskterminated-or-afexception branch December 19, 2024 16:18
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:task-execution-interface-aip72 AIP-72: Task Execution Interface (TEI) aka Task SDK area:task-sdk

Development

Successfully merging this pull request may close these issues.

2 participants