hankehly commented on code in PR #27564:
URL: https://github.com/apache/airflow/pull/27564#discussion_r1018482000


##########
tests/providers/amazon/aws/log/test_cloudwatch_task_handler.py:
##########
@@ -154,49 +154,22 @@ def test_read(self):
             [{"end_of_log": True}],
         )
 
-    def test_read_wrong_log_stream(self):
-        generate_log_events(
-            self.conn,
-            self.remote_log_group,
-            "alternate_log_stream",
-            [
-                {"timestamp": 10000, "message": "First"},
-                {"timestamp": 20000, "message": "Second"},
-                {"timestamp": 30000, "message": "Third"},
-            ],
-        )
-
-        msg_template = "*** Reading remote log from Cloudwatch log_group: {} 
log_stream: {}.\n{}\n"
-        error_msg = (
-            "Could not read remote logs from log_group: "
-            f"{self.remote_log_group} log_stream: {self.remote_log_stream}."
-        )
-        assert self.cloudwatch_task_handler.read(self.ti) == (
-            [[("", msg_template.format(self.remote_log_group, 
self.remote_log_stream, error_msg))]],
-            [{"end_of_log": True}],
-        )
-
-    def test_read_wrong_log_group(self):
-        generate_log_events(
-            self.conn,
-            "alternate_log_group",
-            self.remote_log_stream,
-            [
-                {"timestamp": 10000, "message": "First"},
-                {"timestamp": 20000, "message": "Second"},
-                {"timestamp": 30000, "message": "Third"},
-            ],
-        )
-
-        msg_template = "*** Reading remote log from Cloudwatch log_group: {} 
log_stream: {}.\n{}\n"
-        error_msg = (
-            f"Could not read remote logs from log_group: "
-            f"{self.remote_log_group} log_stream: {self.remote_log_stream}."
-        )
-        assert self.cloudwatch_task_handler.read(self.ti) == (
-            [[("", msg_template.format(self.remote_log_group, 
self.remote_log_stream, error_msg))]],
-            [{"end_of_log": True}],
+    def test_should_read_from_local_on_failure_to_fetch_remote_logs(self):
+        """Check that local logs are displayed on failure to fetch remote 
logs"""
+        self.cloudwatch_task_handler.set_context(self.ti)
+        with mock.patch.object(self.cloudwatch_task_handler, 
"get_cloudwatch_logs") as mock_get_logs:
+            mock_get_logs.side_effect = Exception("Failed to connect")
+            log, metadata = self.cloudwatch_task_handler._read(self.ti, 
self.ti.try_number)
+        expected_log = (
+            f"*** Unable to read remote logs from Cloudwatch (log_group: 
{self.remote_log_group}, "
+            f"log_stream: {self.remote_log_stream})\n*** Failed to connect\n\n"
+            # The value of "log_pos" is equal to the length of this next line
+            f"*** Reading local file: 
{self.local_log_location}/{self.remote_log_stream}\n"
         )
+        assert log == expected_log
+        expected_log_pos = 26 + len(self.local_log_location) + 
len(self.remote_log_stream)

Review Comment:
   Log position seems irrelevant in the context of the CloudWatch handler 
because we fetch/return all logs in one request. I'm following the example I 
found in the GCS task handler here.
   
   
https://github.com/apache/airflow/blob/21063267fd9764b2ca38669e8faec75d9b87179c/tests/providers/google/cloud/log/test_gcs_task_handler.py#L108



-- 
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]

Reply via email to