dstandish commented on a change in pull request #13832:
URL: https://github.com/apache/airflow/pull/13832#discussion_r571387904
##########
File path: airflow/providers/amazon/aws/operators/batch.py
##########
@@ -177,29 +177,26 @@ def submit_job(self, context: Dict): # pylint:
disable=unused-argument
self.job_id = response["jobId"]
self.log.info("AWS Batch job (%s) started: %s", self.job_id,
response)
-
except Exception as e:
self.log.error("AWS Batch job (%s) failed submission", self.job_id)
raise AirflowException(e)
def monitor_job(self, context: Dict): # pylint: disable=unused-argument
"""
Monitor an AWS Batch job
+ monitor_job can raise an exception or an AirflowTaskTimeout can be
raised if execution_timeout
+ is given while creating the task. These exceptions should be handled
in taskinstance.py
+ instead of here like it was previously done
Review comment:
ah yes makes sense. sorry to consume so much of your time. and thanks
for explaining.
when i did "find in files" i saw this datasync reference but assumed
datasync would not be involved in batch hook 🤔.
in any case i am glad to see this change. we should never catch Exception
and reraise with AirflowException (i.e. catch and reraise for no reason) and
this is a good example why... i see it somewhat frequently... it only discards
information and adds indent.
on the related question of task timeout exception... curious... i wonder if
this exception should be reserved only for an `execution_timeout` timeout, and
thou shalt not use it otherwise? seems like it to me... i don't think there
would be any different outcome for the task if it was `raise Exception('i give
up')` -- i.e. there's no special handling behavior e.g. like there is with skip
and reschedule is there?
----------------------------------------------------------------
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]
