-
Notifications
You must be signed in to change notification settings - Fork 16.4k
feat(task_instances): guard ti update state and set task to fail if exception encountered #51295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
amoghrajesh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Lee-W could you please update the PR description with the intent of why you are making this change?
airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py
Outdated
Show resolved
Hide resolved
it should be a draft one. basically what we discussed this afternoon except for changing task runner is not correct. will update the descipriont after late dinner |
8762ed8 to
e724fd4
Compare
|
It would be nice if we could get this one and #50654 reviewed. IMO, the other one is more urgent and important |
0f08675 to
022fd63
Compare
airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py
Outdated
Show resolved
Hide resolved
airflow-core/tests/unit/api_fastapi/execution_api/versions/head/test_task_instances.py
Outdated
Show resolved
Hide resolved
uranusjr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this makes sense. A couple of non-blocking comments.
d534d82 to
3a34038
Compare
|
Looks like the test failure is relevant. |
Yes, I'm now working on it. Basically, we'll need to remove the future rescheulde from data |
|
just rebuild the query to avoid mysql failure |
airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py
Outdated
Show resolved
Hide resolved
0a28e64 to
ac7ffcd
Compare
|
As it has been approved and we're about to start work on releasing 3.0.2, I will merge this one once the CI pass again |
…to fail if exception encountered (#51295) * feat(task_instances): guard ti update state and set to fail if exception enounctered * feat(task_instances): catch mysql error and set the task to fail * test: remove unnecessay check * fix(task_instances): handle mysql error * refactor(task-instances): merge mysql logic back to the original private function (cherry picked from commit b5a3b4e) Co-authored-by: Wei Lee <weilee.rx@gmail.com>
…to fail if exception encountered (#51295) (#51470) * feat(task_instances): guard ti update state and set to fail if exception enounctered * feat(task_instances): catch mysql error and set the task to fail * test: remove unnecessay check * fix(task_instances): handle mysql error * refactor(task-instances): merge mysql logic back to the original private function (cherry picked from commit b5a3b4e) Co-authored-by: Wei Lee <weilee.rx@gmail.com>
…to fail if exception encountered (#51295) (#51470) * feat(task_instances): guard ti update state and set to fail if exception enounctered * feat(task_instances): catch mysql error and set the task to fail * test: remove unnecessay check * fix(task_instances): handle mysql error * refactor(task-instances): merge mysql logic back to the original private function (cherry picked from commit b5a3b4e) Co-authored-by: Wei Lee <weilee.rx@gmail.com>
…xception encountered (apache#51295) * feat(task_instances): guard ti update state and set to fail if exception enounctered * feat(task_instances): catch mysql error and set the task to fail * test: remove unnecessay check * fix(task_instances): handle mysql error * refactor(task-instances): merge mysql logic back to the original private function
Why
closes: #50654
If an unexpected error occurs during task update, the API endpoint crashes with an unhandled exception, and the task state remains, causing the task to hang.
What
Set the task to fail if such case happens
^ 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.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.