potiuk commented on a change in pull request #19662:
URL: https://github.com/apache/airflow/pull/19662#discussion_r751686441
##########
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:
I do not yet know :( .. My first thought is though - how to make sure
that "crucial" functionality does not depend on the context being provided.
Basically every on_kill implementation that will use `context` will have to
do something like that:
```
if context:
# do this and perform some actions
else:
# hmmmm what should we do there if context is missing?
```
My initial thought is;
a) how to make sure that every operator does the if context (bringing back -
currently disabled - MyPy and adding Context as TypedDict derivation migh help
with that)
b) how to make sure that the "# do this and perform some actions" does not
become "actually mandatory",
For the b) I can imagine that you will implement some handling in the "if
context" that will render such operator virtually useless when context is None.
Such operators can leak memory, leave the pods behind etc. etc. And while those
operators will be "technically compatible" with Airlfow < 2.3.0, they will
provide a very poor experience there.
I have no answers yet, This is just a feeling that we somehow have to
address those two problems.
--
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]
