Guaqamole commented on code in PR #44320:
URL: https://github.com/apache/airflow/pull/44320#discussion_r1860403615


##########
providers/src/airflow/providers/amazon/aws/transfers/sftp_to_s3.py:
##########
@@ -85,6 +89,14 @@ def execute(self, context: Context) -> None:
 
         sftp_client = ssh_hook.get_conn().open_sftp()
 
+        try:
+            sftp_client.stat(self.sftp_path)
+        except FileNotFoundError:
+            if self.fail_on_file_not_exist:
+                raise
+            self.log.info("File %s not found on SFTP server. Skipping 
transfer.", self.sftp_path)
+            return

Review Comment:
   @vincbeck 
   the purpose of this implementation is to give users an option _not failing_ 
the dag.
   
   setting `fail_on_file_not_exist` to `False` means the user is trying to skip 
the task and get informed about FileNotFound case, while _not failing_ the dag.
   
   just like @ferruzzi said, the code does look much cleaner but users won't be 
able to know if the task succeeded because the file existed or just skipped 
task even if the file did not exist.
   
   in my opinion **its better to have logging** so that users can be informed 
about the file existence.



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

Reply via email to