hussein-awala commented on code in PR #35105:
URL: https://github.com/apache/airflow/pull/35105#discussion_r1367884193


##########
airflow/providers/ftp/hooks/ftp.py:
##########
@@ -77,8 +77,10 @@ def describe_directory(self, path: str) -> dict:
         :param path: full path to the remote directory
         """
         conn = self.get_conn()
+        current_path = conn.pwd()
         conn.cwd(path)
         files = dict(conn.mlsd())
+        conn.cwd(current_path)

Review Comment:
   What about providing the path to `mlsd` method instead? 
(https://pythontic.com/ftplib/ftp/mlsd):
    ```suggestion
            files = dict(conn.mlsd(path=path))
   ```



##########
airflow/providers/ftp/hooks/ftp.py:
##########
@@ -182,10 +185,12 @@ def write_to_file_with_progress(data):
             callback = output_handle.write
 
         remote_path, remote_file_name = os.path.split(remote_full_path)
+        current_path = conn.pwd()
         conn.cwd(remote_path)
         self.log.info("Retrieving file from FTP: %s", remote_full_path)
         conn.retrbinary(f"RETR {remote_file_name}", callback, block_size)

Review Comment:
   Here you can provide the path of the file instead of its name to the command.



##########
airflow/providers/ftp/hooks/ftp.py:
##########
@@ -214,8 +219,10 @@ def store_file(
         else:
             input_handle = local_full_path_or_buffer
         remote_path, remote_file_name = os.path.split(remote_full_path)
+        current_path = conn.pwd()
         conn.cwd(remote_path)
         conn.storbinary(f"STOR {remote_file_name}", input_handle, block_size)

Review Comment:
   and here you can provide the path instead of the name too



##########
airflow/providers/ftp/hooks/ftp.py:
##########
@@ -88,9 +90,10 @@ def list_directory(self, path: str) -> list[str]:
         :param path: full path to the remote directory to list
         """
         conn = self.get_conn()
+        current_path = conn.pwd()
         conn.cwd(path)
-
         files = conn.nlst()
+        conn.cwd(current_path)

Review Comment:
   same here (https://pythontic.com/ftplib/ftp/nlst):
   ```suggestion
           files = conn.nlst(path=path)
   ```



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