jstern commented on a change in pull request #7133: [AIRFLOW-6535] add
exception to fail without retry
URL: https://github.com/apache/airflow/pull/7133#discussion_r374817019
##########
File path: airflow/models/taskinstance.py
##########
@@ -1124,64 +1129,56 @@ def handle_failure(self, error, test_mode=None,
context=None, session=None):
if context is not None:
context['exception'] = error
- # Let's go deeper
- try:
Review comment:
To give a more complete answer, here is the try/except in master. I've
elided some arguments to log.info and added comments to the two lines where an
exception being raised **would** result in an accurately logged error message.
Exceptions raised on other lines can cause us to log a message about failing to
send an email when that's not what actually happened.
```python
# Let's go deeper
try:
# Since this function is called only when the TaskInstance state
is running,
# try_number contains the current try_number (not the next). We
# only mark task instance as FAILED if the next task instance
# try_number exceeds the max_tries.
if self.is_eligible_to_retry():
self.state = State.UP_FOR_RETRY
self.log.info('Marking task as UP_FOR_RETRY')
if task.email_on_retry and task.email:
self.email_alert(error) # RELEVANT TO EXCEPT BLOCK
else:
self.state = State.FAILED
if task.retries:
self.log.info(...)
else:
self.log.info(...)
if task.email_on_failure and task.email:
self.email_alert(error) # RELEVANT TO EXCEPT BLOCK
except Exception as e2:
self.log.error('Failed to send email to: %s', task.email)
self.log.exception(e2)
```
And here is the more tightly-scoped version in this PR:
```python
if email_for_state and task.email:
try:
self.email_alert(error)
except Exception as e2:
self.log.error('Failed to send email to: %s', task.email)
self.log.exception(e2)
```
While https://github.com/apache/airflow/pull/7257 addresses the only
non-email failure I've personally seen happen in practice, this PR does put
some lines of code that were previously inside of a try/except on the outside
(and I'm sure what I've personally seen happen is a fraction of what actually
might). Please let me know if this increased chance of other exceptions
bubbling up uncaught is unacceptable and I'll look to restructure while trying
to preserve accurate logging....
----------------------------------------------------------------
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]
With regards,
Apache Git Services
