-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
Fix logic to cancel the external job if the TaskInstance is not in a running or deferred state for BigQueryInsertJobOperator #39442
Conversation
7d5cbed
to
2e9a14d
Compare
@thesuperzapper please take a look |
2e9a14d
to
b265f02
Compare
Just have a quick discussion with @sunank200. We'll yield the event if CancelError was raised , and handle the TaskInstance state check and cancelation in |
This approach would not work as execute on complete is not called when the task is manually cancelled when inside triggerer |
04e55ee
to
b71e41c
Compare
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.
LGTM. Some minor suggestions inline
b71e41c
to
8a24aa7
Compare
9335924
to
7b2af60
Compare
@thesuperzapper Please take a look I have tested this and it works fine. |
7b2af60
to
35d61c2
Compare
35d61c2
to
770010a
Compare
…running or deferred state for BigQueryInsertJobOperator (apache#39442)
…running or deferred state for BigQueryInsertJobOperator (apache#39442)
PR #38912 introduces a method for handling asyncio.CancelledError in a try/except block. However, this method is deemed unsafe, and it affects
BigQueryInsertJobOperator
operators, which enables external job cancellation if the triggerer restarts or crashes. This can cause weird behaviour like rescheduling deferred operators, as Airflow remains unaware of job cancellations.As a workaround, capturing
asyncio.CancelledError
cancels the job only if the TaskInstance is not in a running or deferred state. This prevents premature external job termination.More details at: #36090 (comment)
^ 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.