kaxil commented on code in PR #73253:
URL: https://github.com/apache/airflow/pull/73253#discussion_r4093109020
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -2086,7 +2086,7 @@ def _run_task_state_change_callbacks(
for i, callback in enumerate(getattr(task, kind)):
try:
create_executable_runner(callback,
context_get_outlet_events(context), logger=log).run(context)
- except Exception:
+ except (Exception, SystemExit):
Review Comment:
Adding a concrete case to this: `on_execute_callback` goes through the same
helper, and it runs before `execute()`. Before this change a `sys.exit()` there
escaped to the `except SystemExit` in `_run_task_and_map_outcome`, so the
attempt failed or retried without running the operator. Now it's logged and
`execute()` runs anyway. The new `SystemExit` parametrization of
`test_task_runner_not_fail_on_failed_callback` asserts exactly that for the
success case (`"execute success"` after the callback exited).
The retry report doesn't seem to need the broader catch. The pending
`RetryTask` is sent at exit whatever the exit code, and in-process the
`finally` in `InProcessTestSupervisor.start` covers it
(`test_pending_retry_is_reported_when_finalization_raises` already has a
`SystemExit` case). Could the `SystemExit` catch be limited to the finalization
kinds, so `on_execute_callback` keeps aborting the attempt?
--
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]
