potiuk commented on a change in pull request #19662:
URL: https://github.com/apache/airflow/pull/19662#discussion_r752556309
##########
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:
One other comment here:
The "metaclass" thingie is of course very complex and I am not really sure
if the case is 'strong enough' for it. For me also adding the > 2.3.0 limits
automatically for providers that use "on_kill(context) is acceptable approach.
It's also way simpler and faster to implement and test.
It also has the additional benefit, that we can gently "push" the users to
newer Airflow versions - which is generally speaking a "nice" side-effect of
such limits.
But if we go with "bumping the min airflow version" approach - one thing to
consider is actually using a different method and leave `on_kill` as is.
For example we could add a new `on_terminate(context)` method that will only
be in Airlfow 2.3.0+. That would be more explicit and difficult to miss - and
that would be a real "new feature" added, rather than
`kind-a-compatible-augmented-old-feature`. I think that might be far less
confusing and easier to explain.
--
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]
