vandonr-amz commented on code in PR #34570:
URL: https://github.com/apache/airflow/pull/34570#discussion_r1336130248
##########
airflow/providers/amazon/aws/operators/batch.py:
##########
@@ -359,7 +359,12 @@ def monitor_job(self, context: Context):
else:
self.hook.wait_for_job(self.job_id)
- awslogs = self.hook.get_job_all_awslogs_info(self.job_id)
+ awslogs = []
+ try:
+ awslogs = self.hook.get_job_all_awslogs_info(self.job_id)
+ except AirflowException as ae:
+ self.log.warning("Cannot determine where to find the AWS logs for
this Batch job: %s", ae)
Review Comment:
honestly, I think a test for this would be overkill.
It's a 6 lines change, and there is really very little room for a mistake in
it, there is no logic, it's just a try/except.
Adding a test would make this commit 10x bigger, with no added benefit imho.
It's not like someone would come across this code and remove the try/except
block thinking it's useless, only to be caught by the test. It's already
documented in place by the text of the warning log.
--
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]
