MaksYermak commented on code in PR #49768:
URL: https://github.com/apache/airflow/pull/49768#discussion_r2745426788


##########
providers/google/src/airflow/providers/google/cloud/transfers/s3_to_gcs.py:
##########
@@ -335,7 +336,15 @@ def submit_transfer_jobs(self, files: list[str], gcs_hook: 
GCSHook, s3_hook: S3H
 
         return job_names
 
-    def execute_complete(self, context: Context, event: dict[str, Any]) -> 
None:
+    @overload
+    def execute_complete(self, context: Context, event: dict[str, Any], files: 
None) -> None: ...
+    @overload
+    def execute_complete(
+        self, context: Context, event: dict[str, Any], files: Iterable[str]
+    ) -> list[str]: ...
+    def execute_complete(
+        self, context: Context, event: dict[str, Any], files: Iterable[str] | 
None = None
+    ) -> list[str] | None:

Review Comment:
   @AlexisBRENON as I understand you want to overload the `execute_complete` 
for it being possible to use the `files` variable from `self.defer` please 
correct me if I am wrong?
   
   I think it is better to put `files` as attribute to 
`CloudStorageTransferServiceCreateJobsTrigger` and, then, return it with 
`success` event 
[here](https://github.com/apache/airflow/blob/main/providers/google/src/airflow/providers/google/cloud/triggers/cloud_storage_transfer_service.py#L121C15-L126C18).
  In that case the operator returns file names only when the transfer's 
operation is successful. What do you think about this idea?



##########
providers/google/src/airflow/providers/google/cloud/transfers/s3_to_gcs.py:
##########
@@ -281,6 +281,7 @@ def transfer_files_async(self, files: list[str], gcs_hook: 
GCSHook, s3_hook: S3H
                 poll_interval=self.poll_interval,
             ),
             method_name="execute_complete",
+            kwargs=dict(files=files),

Review Comment:
   @AlexisBRENON could you please clarify what behavior you want to achieve by 
this line?
   
   If you want to put `files` to `trigger` or return it from `execute_complete` 
then it is better to add it as an attribute to 
`CloudStorageTransferServiceCreateJobsTrigger`.



-- 
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]

Reply via email to