mobuchowski commented on code in PR #39513:
URL: https://github.com/apache/airflow/pull/39513#discussion_r1596535771
##########
tests/listeners/class_listener.py:
##########
@@ -47,9 +47,7 @@ def on_task_instance_success(self, previous_state,
task_instance, session):
self.state.append(TaskInstanceState.SUCCESS)
@hookimpl
- def on_task_instance_failed(
- self, previous_state, task_instance, error: None | str |
BaseException, session
- ):
+ def on_task_instance_failed(self, previous_state, task_instance, session):
Review Comment:
@potiuk True, that reproduction works, but I think we have an expectation
mismatch here. The backwards compatibility here works in other direction - I
would formulate it as it allows plugins to opt-in to receive earlier and later
events.
As the example in [this comment
shows](https://github.com/apache/airflow/pull/38155#issuecomment-2044703627) we
haven't broken earlier listeners by adding this field. If listener wants to add
support to receiving that field, and simultaneously support earlier Airflow
versions, it's sufficient to do that:
```python
if AIRFLOW_VERSION >= Version("2.10.0"):
@hookimpl
def on_task_instance_failed(previous_state, task_instance, error: None |
str | BaseException, session):
...
else:
@hookimpl
def on_task_instance_failed(previous_state, task_instance, session):
...
```
Is this somehow wrong?
--
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]
