Skip to content

Conversation

@Dawnpool
Copy link
Contributor

@Dawnpool Dawnpool commented Jan 29, 2025

This PR adds DELETE operation in SFTPOperator, related to the issue(#40365) I have raised before.

Currently, SFTPOperator only supports PUT and GET operations. With this update, you can now delete files or directories on a remote SFTP server by putting paths in the remote_filepath parameter.

NOTE: The local_filepath parameter does nothing in DELETE operation even if it's provided

closes: #40365

@eladkal
Copy link
Contributor

eladkal commented Jan 30, 2025

NOTE: The local_filepath parameter does nothing in DELETE operation even if it's provided

Then you need to identify this case in the code and raise exception. It's best to tell users what is not right rather then ask them to read documentation.

example:

if soft_fail is True and never_fail is True:
raise ValueError("soft_fail and never_fail are mutually exclusive, you can not provide both.")

(Don't forget to add test case to cover this case)

Alternatively, if preferred, you can just log that local_filepath was ignored (with some informative message).

@Dawnpool
Copy link
Contributor Author

Dawnpool commented Feb 3, 2025

Thanks for your review @eladkal
Just added an exception raise and test code for it. Please let me know if you have any more concerns.

@Dawnpool
Copy link
Contributor Author

Dawnpool commented Feb 4, 2025

Thanks for your review @morooshka
I've made changes based on your feedback in this commit.
Feel free to let me know if you have any more feedback :)

@potiuk
Copy link
Member

potiuk commented Feb 6, 2025

Looks cool. Thanks @morooshka for all the valuable comments !

@potiuk potiuk merged commit ea4b6c2 into apache:main Feb 6, 2025
61 checks passed
insomnes pushed a commit to insomnes/airflow that referenced this pull request Feb 6, 2025
* Add DELETE operation in SFTPOperator

* Add test code

* Prevent local_filepath usage for DELETE operation

* Update SFTPOperator with changes in condititon check and error msg

* Update error message
niklasr22 pushed a commit to niklasr22/airflow that referenced this pull request Feb 8, 2025
* Add DELETE operation in SFTPOperator

* Add test code

* Prevent local_filepath usage for DELETE operation

* Update SFTPOperator with changes in condititon check and error msg

* Update error message
ambika-garg pushed a commit to ambika-garg/airflow that referenced this pull request Feb 17, 2025
* Add DELETE operation in SFTPOperator

* Add test code

* Prevent local_filepath usage for DELETE operation

* Update SFTPOperator with changes in condititon check and error msg

* Update error message
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve SFTPOperator with directory transfer and DELETE operation

4 participants