houqp commented on a change in pull request #10917:
URL: https://github.com/apache/airflow/pull/10917#discussion_r511694126
##########
File path: airflow/models/taskinstance.py
##########
@@ -1345,11 +1354,12 @@ def handle_failure(self, error, test_mode=None,
context=None, force_fail=False,
# Log failure duration
session.add(TaskFail(task, self.execution_date, self.start_date,
self.end_date))
- if context is not None:
- context['exception'] = error
Review comment:
this is the biggest semantic change this PR introduces, failure callback
won't have access to `context['exception']` anymore because it's not called
from a separate task manager process. It is unavoidable because if we don't
execute callback from external monitor process, then we won't be able to catch
task failures caused by out of memory or segfault.
I think this is a reasonable tradeoff because some of the success/failure
callbacks were already invoked from external manager process when states are
updated externally, therefore, the contract for accessing
`context['exception']` is already partially broken today.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
