[GitHub] [airflow] vandonr-amz commented on a diff in pull request #33231: Improve fetching logs from AWS

["vandonr-amz (via GitHub)" <gi...@apache.org>]

vandonr-amz commented on code in PR #33231:
URL: https://github.com/apache/airflow/pull/33231#discussion_r1288948715


##########
airflow/providers/amazon/aws/log/cloudwatch_task_handler.py:
##########
@@ -103,17 +106,29 @@ def _read(self, task_instance, try_number, metadata=None):
             log += local_log
             return log, metadata
 
-    def get_cloudwatch_logs(self, stream_name: str) -> str:
+    def get_cloudwatch_logs(self, stream_name: str, task_instance: TaskInstance) -> str:
         """
         Return all logs from the given log stream.
 
         :param stream_name: name of the Cloudwatch log stream to get all logs from
+        :param task_instance: the task instance to get logs about
         :return: string of all logs from the given log stream
         """
+        start_time = (
+            None if task_instance.start_date is None else datetime_to_epoch_utc_ms(task_instance.start_date)

Review Comment:
   nit: `start_time = None` violates type annotations (it's supposed to be an `int`, not an `int | None`)



##########
airflow/providers/amazon/aws/hooks/logs.py:
##########
@@ -72,7 +74,9 @@ def get_log_events(
 
         :param log_group: The name of the log group.
         :param log_stream_name: The name of the specific stream.
-        :param start_time: The time stamp value to start reading the logs from (default: 0).
+        :param start_time: The timestamp value to start reading the logs from (default: 0).

Review Comment:
   ```suggestion
           :param start_time: The timestamp value in ms to start reading the logs from (default: 0).
   ```



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org