moiseenkov commented on code in PR #29462:
URL: https://github.com/apache/airflow/pull/29462#discussion_r1116734826
##########
airflow/providers/google/cloud/transfers/s3_to_gcs.py:
##########
@@ -104,6 +127,7 @@ class S3ToGCSOperator(S3ListOperator):
"google_impersonation_chain",
)
ui_color = "#e09411"
+ transfer_job_max_files_number = 1000
Review Comment:
Thank you for noticing. It's a good point, but I think it's better to leave
it hard-coded, because:
1. This attribute exist only because of Google Storage Transfer Service API
limitation for the field `includePrefixes[]` (docs:
https://cloud.google.com/storage-transfer/docs/reference/rest/v1/TransferSpec#ObjectConditions).
We can't increase it, because it might cause "400 Bad request" responses from
the API, and decreasing it is also not a good idea, because it might increase
the number of API calls. That's why I don't see a use case for changing it,
unless the API updates.
2. Moving this attribute into the `S3ToGCSOperator`'s parameters list also
doesn't look good to me, because it is a part of a low level logic that is out
of the operator's scope. This new parameter won't influence the operator's
result and thus will be quite useless for users. Moreover, after creating a new
parameter we will have to maintain it, and it won't be easy to remove it in the
future.
That's why from my perspective exposing it to the users won't bring any
value. However, it is always possible to monkey patch it if needed. WDYT?
--
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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
