[GitHub] [airflow] ashb commented on a change in pull request #9908: Add extra error handling to S3 remote logging (#9118)

[GitBox <gi...@apache.org>]

ashb commented on a change in pull request #9908:
URL: https://github.com/apache/airflow/pull/9908#discussion_r522850346



##########
File path: tests/providers/amazon/aws/log/test_s3_task_handler.py
##########
@@ -143,12 +144,27 @@ def test_read_when_s3_log_missing(self):
         self.assertIn('*** Log file does not exist:', log[0][0][-1])
         self.assertEqual({'end_of_log': True}, metadata[0])
 
+    def test_s3_read_when_log_missing(self):
+        handler = self.s3_task_handler
+        url = 's3://bucket/foo'
+        with mock.patch.object(handler.log, 'error') as mock_error:
+            result = handler.s3_read(url, return_error=True)
+            msg = (
+                f'Could not read logs from {url} with error: An error occurred (404) when calling the '
+                f'HeadObject operation: Not Found '
+            )
+            self.assertEqual(result, msg)
+            mock_error.assert_called_once_with(msg, exc_info=True)
+
     def test_read_raises_return_error(self):
         handler = self.s3_task_handler
         url = 's3://nonexistentbucket/foo'
         with mock.patch.object(handler.log, 'error') as mock_error:
             result = handler.s3_read(url, return_error=True)
-            msg = 'Could not read logs from %s' % url
+            msg = (
+                f'Could not read logs from {url} with error: An error occurred (NoSuchBucket) when '
+                f'calling the HeadObject operation: The specified bucket does not exist '

Review comment:
       ```suggestion
                   f'calling the HeadObject operation: The specified bucket does not exist'
   ```

##########
File path: tests/providers/amazon/aws/log/test_s3_task_handler.py
##########
@@ -143,12 +144,27 @@ def test_read_when_s3_log_missing(self):
         self.assertIn('*** Log file does not exist:', log[0][0][-1])
         self.assertEqual({'end_of_log': True}, metadata[0])
 
+    def test_s3_read_when_log_missing(self):
+        handler = self.s3_task_handler
+        url = 's3://bucket/foo'
+        with mock.patch.object(handler.log, 'error') as mock_error:
+            result = handler.s3_read(url, return_error=True)
+            msg = (
+                f'Could not read logs from {url} with error: An error occurred (404) when calling the '
+                f'HeadObject operation: Not Found '

Review comment:
       ```suggestion
                   f'HeadObject operation: Not Found'
   ```




----------------------------------------------------------------
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:
users@infra.apache.org