dstandish commented on a change in pull request #13832:
URL: https://github.com/apache/airflow/pull/13832#discussion_r570705738
##########
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:
what was done previously is generally something that is only of interest
in the PR -- not in the code. in this case i'd just document what it does
_now_
and in this case, i would not add any documentation
in general it is assumed that code can throw exceptions. that a task
timeout can be raised is true of all operators. and you don't need to add a
warning when you are not catching an exception
##########
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:
what was done previously is generally something that is only of interest
in the PR -- not in the code. in this case i'd just document what it does
_now_
but in this case, i would not add any documentation
in general it is assumed that code can throw exceptions. that a task
timeout can be raised is true of all operators. and you don't need to add a
warning when you are not catching an exception
----------------------------------------------------------------
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]
