Skip to content

Conversation

@kacpermuda
Copy link
Contributor

Now we are calling get_listener_manager().hook.on_task_instance_failed with ti that still has the RUNNING state assigned. By using only ti.state, there is no way of knowing if it failed or not, it looks the same as the one being passed to get_listener_manager().hook.on_task_instance_start.

We should probably move the listener call to after, we assign the FAILED or UP_FOR_RETRY state, so that we can use that information. I've tested this change and it looks like there is no influence on any other part of code, but maybe I'm missing something.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

Signed-off-by: Kacper Muda <mudakacper@gmail.com>
@potiuk
Copy link
Member

potiuk commented Jul 26, 2024

Makes sense. But would love other look :)

@mobuchowski mobuchowski merged commit ad30ba1 into apache:main Aug 1, 2024
@kacpermuda kacpermuda deleted the fix-listener-on-failure branch August 1, 2024 12:11
@utkarsharma2 utkarsharma2 added the type:improvement Changelog: Improvements label Aug 1, 2024
@utkarsharma2 utkarsharma2 added this to the Airflow 2.10.0 milestone Aug 1, 2024
@jedcunningham jedcunningham added type:bug-fix Changelog: Bug Fixes and removed type:improvement Changelog: Improvements labels Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug-fix Changelog: Bug Fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants