yuseok89 commented on code in PR #65724:
URL: https://github.com/apache/airflow/pull/65724#discussion_r3178291830
##########
providers/google/src/airflow/providers/google/cloud/transfers/s3_to_gcs.py:
##########
@@ -210,6 +210,9 @@ def _get_files(self, context: Context, gcs_hook: GCSHook)
-> list[str]:
# use the super method to list all the files in an S3 bucket/key
s3_objects = super().execute(context)
+ # Skip S3 folder-marker keys (``.../``); they overlap sibling keys in
Storage Transfer includePrefixes.
+ s3_objects = [obj for obj in s3_objects if not obj.endswith("/")]
Review Comment:
Thanks for the review, and agreed that the previous patch was too broad.
That said, I'd push back on keeping zero-byte directory markers in the XCom
return at all. Just my personal take, but I don't think the original author
intentionally exposed those keys via XCom. I'd like the XCom output to also be
valid, meaning only the data objects this operator actually transferred.
There's a reliable way to do that without breaking your concern about real
trailing-slash keys. If a listed key is a strict prefix of another listed key
in the same super().execute(context) result, it's almost certainly a directory
marker, because its children are in the very same listing.
So the proposal is to drop only entries that meet both conditions.
1. Strict prefix of another listed key in the same listing, and
2. Ends with /.
Applied once in `_get_files`, so deferrable, non-deferrable, and XCom all
see the same cleaned-up list. A lone trailing-slash key with no overlap stays
kept. The STS includePrefixes sibling-sweep you originally flagged is fixed as
a side effect.
Sound good? If so I'll push the updated patch.
--
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]
