potiuk commented on a change in pull request #19662:
URL: https://github.com/apache/airflow/pull/19662#discussion_r751736159
##########
File path: airflow/models/baseoperator.py
##########
@@ -1000,7 +1000,7 @@ def post_execute(self, context: Any, result: Any = None):
if self._post_execute_hook is not None:
self._post_execute_hook(context, result)
- def on_kill(self) -> None:
Review comment:
Another idea (maybe a bit better).
Is it always the case (I am thinking about defferable operators here) that
on_kill() is executed always after execute() for the same instance of
operator?
If the answer is `yes`, then maybe we can figure out a scheme (using
BaseOperator meta-classes similarly as with apply_defaults) to make context
non-optional?
If the `on_kill(context)` operator is instantiated for Airflow <2.3 the
meta-class could monkey-patch the execute() and on_kill() methods of that
operator. It could 'catch' the execute() method call and store the `context` as
attribute of the operator instance, and add to on_kill() automatically.
This way, the new providers with `on_kill(context)` instaled for Airflow <
2.3 will also get the context passed to their on_kill method.
This would mean that:
* context, when used in on_kill, will not be optional
* operators that have on_kill(context) implemented will not have do `if
context:`
* any sophisticated cleanup implemented for those operators will also work
for Airlfow < 2.3
* the performance penalty for monkeypatching (small) will be only paid for
Airflow < 2.3
* the interface of each operator using on_kill(context) will be even cleaner
because of the non-optionality
The only problem with that solution is if we have a case when on_kill()
method is called for instance of operator without prior execute() call for the
same instance - but I think this is not the case currently (so we should be
safe).
--
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]
