thesuperzapper commented on issue #36090: URL: https://github.com/apache/airflow/issues/36090#issuecomment-2168436868
> Probably worth implementing it on the "airflow" level as you mentioned. Ate you willing to submit a PR on that @thesuperzapper ? Did I understand well? @potiuk I can definitely help with _reviewing_ PR to implement the workaround above into existing deferred operators that use `on_kill`, because all of them need to be updated. In terms of implementing some kind of solution at the airflow level, I'm happy to discuss possible solutions, but since we have no idea what the implementation would be yet, I can't commit to anything. I guess there are two approaches to a generic solution: 1. Try and get `on_kill` to run for deferred operators, probably by rescheduling it or effectively just running it in the exception handler of the trigger like I'm doing above. (The problem is the `on_kill` method is on the operator, not the triggerer, so it's not available after being deferred) 2. Just document the above workaround in the deferred operator docs, because It does work, and won't require us to set a new minimum airflow version for some providers. (Note, the only problem is in the extremely rare case that the trigger crashes at the exact moment the exception handler is running, which might mean the external job doesn't get killed, but there are already many similar race conditions in airflow) -- 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]
