You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Dawnpool (via GitHub)" <gi...@apache.org> on 2024/11/18 07:47:47 UTC

[PR] Add directory transfer support for SFTPOperator [airflow]

Dawnpool opened a new pull request, #44126:
URL: https://github.com/apache/airflow/pull/44126

   This PR implements directory transfer for SFTPOperator, related to the [issue](https://github.com/apache/airflow/issues/40365) I have raised before.
   Currently, the SFTPOperator only accepts file paths, and you have to specify every filename in a folder by list when transferring an entire folder.
   By adding some directory handling logic, the operator now can accept directory paths as well, allowing users easily transfer entire folders.


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "kunaljubce (via GitHub)" <gi...@apache.org>.
kunaljubce commented on code in PR #44126:
URL: https://github.com/apache/airflow/pull/44126#discussion_r1857898034


##########
providers/src/airflow/providers/sftp/hooks/sftp.py:
##########
@@ -275,6 +276,47 @@ def delete_file(self, path: str) -> None:
         conn = self.get_conn()
         conn.remove(path)
 
+    def retrieve_directory(self, remote_full_path: str, local_full_path: str, prefetch: bool = True) -> None:
+        """
+        Transfer the remote directory to a local location.
+
+        If local_full_path is a string path, the directory will be put
+        at that location.
+
+        :param remote_full_path: full path to the remote directory
+        :param local_full_path: full path to the local directory
+        :param prefetch: controls whether prefetch is performed (default: True)
+        """
+        os.mkdir(local_full_path)

Review Comment:
   @Dawnpool Yeah I am not too much in favour of deleting the existing directory, better left to the user to handle that. Seems fine to raise the error though and stop the process! 



-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2563905558

   All right. I think I found it https://github.com/apache/airflow/pull/45249 - very interesting issue (and your change accidentally revealed it because it selectively run non-db `microsoft.azure` test collection as the first group of tests to run - and it turned out that those tests relied on a side-effect of other tests being run before. 


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2564008659

   OK. The fix is merged. You can rebase again and I :crossed_fingers: that it should work now.


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2482181134

   Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
   Here are some useful points:
   - Pay attention to the quality of your code (ruff, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-pre-commit-hooks) will help you with that.
   - In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
   - Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
   - Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
   - Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
   - Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices).
   - Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
   Apache Airflow is a community-driven project and together we are making it better 🚀.
   In case of doubts contact the developers at:
   Mailing List: dev@airflow.apache.org
   Slack: https://s.apache.org/airflow-slack
   


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "kunaljubce (via GitHub)" <gi...@apache.org>.
kunaljubce commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2483706176

   @Dawnpool You seem to have a whole test non-db test suite failing. Can you check it on your local breeze and see if you can fix it?


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "kunaljubce (via GitHub)" <gi...@apache.org>.
kunaljubce commented on code in PR #44126:
URL: https://github.com/apache/airflow/pull/44126#discussion_r1857895095


##########
providers/src/airflow/providers/sftp/hooks/sftp.py:
##########
@@ -276,6 +277,49 @@ def delete_file(self, path: str) -> None:
         conn = self.get_conn()
         conn.remove(path)
 
+    def retrieve_directory(self, remote_full_path: str, local_full_path: str, prefetch: bool = True) -> None:
+        """
+        Transfer the remote directory to a local location.
+
+        If local_full_path is a string path, the directory will be put
+        at that location.
+
+        :param remote_full_path: full path to the remote directory
+        :param local_full_path: full path to the local directory
+        :param prefetch: controls whether prefetch is performed (default: True)
+        """
+        if Path(local_full_path).is_dir():
+            raise AirflowException(f"{local_full_path} already exists")
+        Path(local_full_path).mkdir(parents=True)
+        files, dirs, _ = self.get_tree_map(remote_full_path)
+        for dir_path in dirs:
+            new_local_path = os.path.join(local_full_path, os.path.relpath(dir_path, remote_full_path))
+            Path(new_local_path).mkdir(parents=True, exist_ok=True)
+        for file_path in files:
+            new_local_path = os.path.join(local_full_path, os.path.relpath(file_path, remote_full_path))
+            self.retrieve_file(file_path, new_local_path, prefetch)
+
+    def store_directory(self, remote_full_path: str, local_full_path: str, confirm: bool = True) -> None:
+        """
+        Transfer a local directory to the remote location.
+
+        If local_full_path is a string path, the directory will be read
+        from that location.
+
+        :param remote_full_path: full path to the remote directory
+        :param local_full_path: full path to the local directory
+        """
+        self.create_directory(remote_full_path)

Review Comment:
   @Dawnpool What happens if this `remote_full_path` is present in the remote location? Did you test this? Maybe add another check here?



-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "Dev-iL (via GitHub)" <gi...@apache.org>.
Dev-iL commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2564362993

   Thank you for your contribution @Dawnpool!
   
   In this implementation, files are downloaded sequentially, right? If so - when downloading folders containing a large number of small files, this approach will be quite inefficient. I think we should consider expanding the method, or adding another one that downloads files concurrently.  
   
   Possibly related to [SFTPHookAsync](https://airflow.apache.org/docs/apache-airflow-providers-sftp/stable/_api/airflow/providers/sftp/hooks/sftp/index.html#airflow.providers.sftp.hooks.sftp.SFTPHookAsync).


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2564133291

   Awesome work, congrats on your first merged pull request! You are invited to check our [Issue Tracker](https://github.com/apache/airflow/issues) for additional contributions.
   


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "Dawnpool (via GitHub)" <gi...@apache.org>.
Dawnpool commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2566131567

   Hi @Dev-iL!
   Yes, just as you said, files are downloaded sequentially since it simply uses multiple calls to the `retrieve_file` function in the order they are collected. I totally agree with your point this approach would be inefficient in the case you said.
   It would be awesome if we could download multiple files concurrently, but I'm not sure what kind of approach to take (probably multi-threading?) and we should definitely do performance checks to compare after implementing the method.


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2505056501

   > Let's wait for one of the maintainers to approve the remaining workflows. That will give you some clarity if rebasing against `main` fixed it. Feel free to drop a gentle reminder on the Slack channel - `#contributors`.
   
   Approved workflows.


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2505182474

   errrors, errors everywhere :D


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "kunaljubce (via GitHub)" <gi...@apache.org>.
kunaljubce commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2506510376

   @Dawnpool Try reproducing your CI tests locally and figuring out what's going wrong. Seems this is a good place to start - https://github.com/apache/airflow/blob/main/dev/breeze/doc/ci/08_running_ci_locally.md


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2564133320

   Finally!


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #44126:
URL: https://github.com/apache/airflow/pull/44126


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2563560360

   Can you please rebase/resolve conflicts and ping me if it fails again.


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2563702881

   I think this should fix it: https://github.com/apache/airflow/pull/45244


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "Dawnpool (via GitHub)" <gi...@apache.org>.
Dawnpool commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2559115598

   @potiuk 
   The non-db tests keep failing and it doesn't seem related to the changes I made based on the error logs.
   I noticed your comment(https://github.com/apache/airflow/pull/44625#issuecomment-2522299932) on a recent approved PR with the same error, where you mentioned that it was an unrelated intermittent error. Is there an ongoing issue with the non-db tests?
   
   @eladkal 
   I have resolved the conflicts. Please feel free to check!
   


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "Dawnpool (via GitHub)" <gi...@apache.org>.
Dawnpool commented on code in PR #44126:
URL: https://github.com/apache/airflow/pull/44126#discussion_r1860896988


##########
providers/src/airflow/providers/sftp/hooks/sftp.py:
##########
@@ -275,6 +276,47 @@ def delete_file(self, path: str) -> None:
         conn = self.get_conn()
         conn.remove(path)
 
+    def retrieve_directory(self, remote_full_path: str, local_full_path: str, prefetch: bool = True) -> None:
+        """
+        Transfer the remote directory to a local location.
+
+        If local_full_path is a string path, the directory will be put
+        at that location.
+
+        :param remote_full_path: full path to the remote directory
+        :param local_full_path: full path to the local directory
+        :param prefetch: controls whether prefetch is performed (default: True)
+        """
+        os.mkdir(local_full_path)

Review Comment:
   @kunaljubce I've also updated this check from `isdir()` to `exists()` to make sure both a file and a directory with the same name do not exist.



-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "Dawnpool (via GitHub)" <gi...@apache.org>.
Dawnpool commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2496039947

   > @Dawnpool You seem to have a whole test non-db test suite failing. Can you check it on your local breeze and see if you can fix it?
   
   Fixed it by merging the main branch!


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "Dawnpool (via GitHub)" <gi...@apache.org>.
Dawnpool commented on code in PR #44126:
URL: https://github.com/apache/airflow/pull/44126#discussion_r1860885817


##########
providers/src/airflow/providers/sftp/hooks/sftp.py:
##########
@@ -276,6 +277,49 @@ def delete_file(self, path: str) -> None:
         conn = self.get_conn()
         conn.remove(path)
 
+    def retrieve_directory(self, remote_full_path: str, local_full_path: str, prefetch: bool = True) -> None:
+        """
+        Transfer the remote directory to a local location.
+
+        If local_full_path is a string path, the directory will be put
+        at that location.
+
+        :param remote_full_path: full path to the remote directory
+        :param local_full_path: full path to the local directory
+        :param prefetch: controls whether prefetch is performed (default: True)
+        """
+        if Path(local_full_path).is_dir():
+            raise AirflowException(f"{local_full_path} already exists")
+        Path(local_full_path).mkdir(parents=True)
+        files, dirs, _ = self.get_tree_map(remote_full_path)
+        for dir_path in dirs:
+            new_local_path = os.path.join(local_full_path, os.path.relpath(dir_path, remote_full_path))
+            Path(new_local_path).mkdir(parents=True, exist_ok=True)
+        for file_path in files:
+            new_local_path = os.path.join(local_full_path, os.path.relpath(file_path, remote_full_path))
+            self.retrieve_file(file_path, new_local_path, prefetch)
+
+    def store_directory(self, remote_full_path: str, local_full_path: str, confirm: bool = True) -> None:
+        """
+        Transfer a local directory to the remote location.
+
+        If local_full_path is a string path, the directory will be read
+        from that location.
+
+        :param remote_full_path: full path to the remote directory
+        :param local_full_path: full path to the local directory
+        """
+        self.create_directory(remote_full_path)

Review Comment:
   For now, it would copy the elements in the existing remote location. Let me add another error raising here. Thanks for a good catch.



-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "kunaljubce (via GitHub)" <gi...@apache.org>.
kunaljubce commented on code in PR #44126:
URL: https://github.com/apache/airflow/pull/44126#discussion_r1847001691


##########
providers/src/airflow/providers/sftp/hooks/sftp.py:
##########
@@ -275,6 +276,47 @@ def delete_file(self, path: str) -> None:
         conn = self.get_conn()
         conn.remove(path)
 
+    def retrieve_directory(self, remote_full_path: str, local_full_path: str, prefetch: bool = True) -> None:
+        """
+        Transfer the remote directory to a local location.
+
+        If local_full_path is a string path, the directory will be put
+        at that location.
+
+        :param remote_full_path: full path to the remote directory
+        :param local_full_path: full path to the local directory
+        :param prefetch: controls whether prefetch is performed (default: True)
+        """
+        os.mkdir(local_full_path)

Review Comment:
   @Dawnpool Can we add a check if `local_full_path` exists before trying to create it again? 



-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2559629012

   I think it's a side effect of bad  configuration - likely some initialization issue in some of the skipped provider tests that are missing somewhere. Likely it is mitigated when full tests are run as some other tests are intitializing SQL Alchemy (where we completely should not need it for DB Tests). I will take a look at it later today and try to find out what it is (And it would be great to not merge it till then as we might be eble to see if the problem is fixed when I find it.


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "Dawnpool (via GitHub)" <gi...@apache.org>.
Dawnpool commented on code in PR #44126:
URL: https://github.com/apache/airflow/pull/44126#discussion_r1855465249


##########
providers/src/airflow/providers/sftp/hooks/sftp.py:
##########
@@ -275,6 +276,47 @@ def delete_file(self, path: str) -> None:
         conn = self.get_conn()
         conn.remove(path)
 
+    def retrieve_directory(self, remote_full_path: str, local_full_path: str, prefetch: bool = True) -> None:
+        """
+        Transfer the remote directory to a local location.
+
+        If local_full_path is a string path, the directory will be put
+        at that location.
+
+        :param remote_full_path: full path to the remote directory
+        :param local_full_path: full path to the local directory
+        :param prefetch: controls whether prefetch is performed (default: True)
+        """
+        os.mkdir(local_full_path)

Review Comment:
   @kunaljubce 
   Added it by raising an error if the path already exists. Or, how about just deleting the whole directory and recreating it if it already exists?



-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "Dawnpool (via GitHub)" <gi...@apache.org>.
Dawnpool commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2563642617

   @potiuk 
   it failed again😥 same error with the non-db tests


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "Dawnpool (via GitHub)" <gi...@apache.org>.
Dawnpool commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2496085469

   > @Dawnpool You seem to have a whole test non-db test suite failing. Can you check it on your local breeze and see if you can fix it?
   
   Hi, I guess there was an issue with the test code at the time I forked the repository. It might have been resolved by merging the main branch. There is no problem on my local breeze environment for now.


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "kunaljubce (via GitHub)" <gi...@apache.org>.
kunaljubce commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2499802218

   > > @Dawnpool You seem to have a whole test non-db test suite failing. Can you check it on your local breeze and see if you can fix it?
   > 
   > Hi, I guess there was an issue with the test code at the time I forked the repository. It might have been resolved by merging the main branch. There is no problem on my local breeze environment for now.
   
   Let's wait for one of the maintainers to approve the remaining workflows. That will give you some clarity if rebasing against `main` fixed it. Feel free to drop a gentle reminder on the Slack channel - `#contributors`.


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2553134209

   PR has conflicts that needs to be resolved


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "Dawnpool (via GitHub)" <gi...@apache.org>.
Dawnpool commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2564134453

   Finally! Thank you all😁


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2563806955

   Nope - there is another issue


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2563698061

   :scream: 
   


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add directory transfer support for SFTPOperator [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #44126:
URL: https://github.com/apache/airflow/pull/44126#issuecomment-2563728591

   Let's see - I added the fix on top and we should see if the problem is fixed.


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org