jvstein commented on code in PR #67144:
URL: https://github.com/apache/airflow/pull/67144#discussion_r3283843555
##########
providers/amazon/src/airflow/providers/amazon/aws/log/s3_task_handler.py:
##########
@@ -62,6 +62,8 @@ def upload(self, path: os.PathLike | str, ti: RuntimeTI):
has_uploaded = self.write(log, remote_loc)
if has_uploaded and self.delete_local_copy:
shutil.rmtree(os.path.dirname(local_loc))
+ elif has_uploaded:
+ local_loc.write_text("")
Review Comment:
I think that if an upload fails on a given worker, we still want to append
logs on subsequent runs that land on the same worker. For example, if the task
lands on different workers between pokes, we would still want to re-attempt the
upload of the local log files, even if they arrive out-of-order in the final
file.
In any scenario where the log upload fails and never re-executes on a
worker, we lose logs, but that's a reality with the current code anyway and I'm
not sure what the right solution is.
Maybe the worse problem is around a transient read failure of the log file
from S3, which could trample the log file entirely.
--
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]
