Lee-W commented on code in PR #59753:
URL: https://github.com/apache/airflow/pull/59753#discussion_r2646436714
##########
providers/google/src/airflow/providers/google/cloud/log/gcs_task_handler.py:
##########
@@ -149,11 +151,28 @@ def no_log_found(exc):
exc, "resp", {}
).get("status") == "404"
- def read(self, relative_path: str, ti: RuntimeTI) -> tuple[LogSourceInfo,
LogMessages | None]:
- messages = []
- logs = []
+ def read(self, relative_path: str, ti: RuntimeTI) -> LogResponse:
+ messages, log_streams = self.stream(relative_path, ti)
+ if not log_streams:
+ return messages, None
+
+ logs: list[str] = []
+ try:
+ # for each log_stream, exhaust the generator into a string
+ for log_stream in log_streams:
+ log_content = "".join(line for line in log_stream)
+ logs.append(log_content)
Review Comment:
```suggestion
logs = [
"".join(line for line in log_stream)
for log_stream in log_streams
]
```
##########
providers/google/src/airflow/providers/google/cloud/log/gcs_task_handler.py:
##########
@@ -164,18 +183,29 @@ def read(self, relative_path: str, ti: RuntimeTI) ->
tuple[LogSourceInfo, LogMes
else:
messages.extend(["Found remote logs:", *[f" * {x}" for x in
sorted(uris)]])
else:
- return messages, None
+ return messages, []
try:
for key in sorted(uris):
blob = storage.Blob.from_string(key, self.client)
- remote_log = blob.download_as_bytes().decode()
- if remote_log:
- logs.append(remote_log)
+ stream = blob.open("r")
Review Comment:
Should we use a context manager here?
--
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]
