[GitHub] [airflow] marcosmarxm commented on a change in pull request #15173: read timestamp from Cloudwatch events

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

marcosmarxm commented on a change in pull request #15173:
URL: https://github.com/apache/airflow/pull/15173#discussion_r606660589



##########
File path: airflow/providers/amazon/aws/log/cloudwatch_task_handler.py
##########
@@ -118,7 +121,14 @@ def get_cloudwatch_logs(self, stream_name: str) -> str:
                     log_group=self.log_group, log_stream_name=stream_name, start_from_head=True
                 )
             )
-            return '\n'.join([event['message'] for event in events])
+
+            def event_to_str(event):
+                event_dt = timezone.make_aware(datetime.utcfromtimestamp(event['timestamp'] / 1000.0))
+                formatted_event_dt = timezone.make_naive(event_dt).isoformat(timespec='seconds')
+                message = event['message']
+                return f'[{formatted_event_dt}] {message}'

Review comment:
       any specific reason to create this function in this scope? Could be better create a private method in the main Handler class called 
   ```ptyhon 
   def _event_to_str(event: dict): -> str
   ```
   This will allow create unit tests for the function. 
   
   The event variable have three fields:
   ```
           :return: | A CloudWatch log event with the following key-value pairs:
                    |   'timestamp' (int): The time in milliseconds of the event.
                    |   'message' (str): The log event data.
                    |   'ingestionTime' (int): The time in milliseconds the event was ingested.
   ```
   The third `ingestionTime` wont be use anymore, it's not useful?




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