Ensure TaskInstance.end_date and duration are populated before invoking failure callbacks #52729
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
amoghrajesh
reviewed
Jul 3, 2025
- Set ti.end_date before creating TaskState in exception handlers - Ensures on_failure_callback context has proper end_date and duration - Fixes race condition where callbacks received incomplete TaskInstance data - Add test to verify callback context contains required timing information
prdai
force-pushed
the
bugfix/52630-task-instance-end-date-duration-on-failure
branch
from
387adee to
896d0c1
Compare
eladkal
added
type:bug-fix
ashb
reviewed
Jul 31, 2025
Member
|
Can you check/test that success callbacks also have an end date, and then we can get this merged |
amoghrajesh
reviewed
Aug 4, 2025
Contributor
amoghrajesh
left a comment
amoghrajesh
left a comment
There was a problem hiding this comment.
It would be nice to test success call backs too. Looks good otherwise
Contributor
Author
|
hi @ashb @amoghrajesh, i tested both success and failure callbacks to make sure that they have the end date and duration info, the following are the log results: $ python -m pytest task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks -v
============================= test session starts ==============================
collecting ... collected 11 items
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_calls_callback[success] PASSED [ 9%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_calls_callback[skipped] PASSED [ 18%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_calls_callback[failure] PASSED [ 27%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_calls_callback[retry] PASSED [ 36%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_on_failure_callback_context PASSED [ 45%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_on_success_callback_context PASSED [ 54%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_both_callbacks_have_timing_info PASSED [ 63%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_not_fail_on_failed_callback[success] PASSED [ 72%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_not_fail_on_failed_callback[skipped] PASSED [ 81%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_not_fail_on_failed_callback[failure] PASSED [ 90%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_not_fail_on_failed_callback[retry] PASSED [100%]
========================= 11 passed, 1 warning in 1.93s =========================I had to update the |
amoghrajesh
approved these changes
Aug 13, 2025
Contributor
amoghrajesh
left a comment
amoghrajesh
left a comment
There was a problem hiding this comment.
LGTM +1 because it has been tested.
@ashb wdyt?
Contributor
|
Unrelated failure, retriggering it |
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
github-actions bot
pushed a commit
that referenced
this pull request
Aug 13, 2025
…efore invoking failure callbacks (#52729) * Fix: Set TaskInstance end_date before failure callbacks (#52630) - Set ti.end_date before creating TaskState in exception handlers - Ensures on_failure_callback context has proper end_date and duration - Fixes race condition where callbacks received incomplete TaskInstance data - Add test to verify callback context contains required timing information * Enhance task callbacks to include end_date and duration metrics (cherry picked from commit eda89ae) Co-authored-by: Ranuga Disansa <79456372+Programmer-RD-AI@users.noreply.github.com>
Backport successfully created: v3-0-test
|
kaxil
pushed a commit
that referenced
this pull request
Aug 13, 2025
…efore invoking failure callbacks (#52729) (#54458) * Fix: Set TaskInstance end_date before failure callbacks (#52630) - Set ti.end_date before creating TaskState in exception handlers - Ensures on_failure_callback context has proper end_date and duration - Fixes race condition where callbacks received incomplete TaskInstance data - Add test to verify callback context contains required timing information * Enhance task callbacks to include end_date and duration metrics (cherry picked from commit eda89ae) Co-authored-by: Ranuga Disansa <79456372+Programmer-RD-AI@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change addresses a race condition in the task runner where downstream failure callbacks might observe an uninitialized or stale
end_dateanddurationon theTaskInstance. By explicitly settingti.end_datebefore constructing theTaskStatein all exception handlers, we guarantee that anyon_failure_callbackreceives a fully populated context, preventing confusing debug sessions and user‐defined callback failures.Key Changes:
ti.end_dateearly in bothAirflowFailExceptionand generic exception handlers, before creating theTaskState.durationautomatically by virtue of the populatedend_date, ensuring callbacks can compute and report accurate runtime metrics.test_task_runner_on_failure_callback_context) that verifies bothend_dateand non‐negativedurationare present in the failure callback context.TaskInstancedata, leading to misleading or missing timing information.This PR closes #52630