Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve SFTPOperator with directory transfer and DELETE operation #40365

Open
2 tasks done
Dawnpool opened this issue Jun 21, 2024 · 7 comments · May be fixed by #46233
Open
2 tasks done

Improve SFTPOperator with directory transfer and DELETE operation #40365

Dawnpool opened this issue Jun 21, 2024 · 7 comments · May be fixed by #46233

Comments

@Dawnpool
Copy link
Contributor

Description

Currently, the SFTPOperator in airflow providers does not support directory transfers. You have to specify every filename in a folder if you want to transfer the whole folder. Additionally, the operator supports only PUT and GET methods, not DELETE methods.

I think this operator would be more powerful if it could transfer an entire folder by specifying just the folder name as well as delete files and folders with DELETE method.

Use case/motivation

I want to copy a folder to an SFTP remote server using only the folder name. Before copying, I want to delete the already existing folder on the SFTP remote server to ensure it is overwritten.

Related issues

No response

Are you willing to submit a PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@Dawnpool Dawnpool added kind:feature Feature Requests needs-triage label for new issues that we didn't triage yet labels Jun 21, 2024
Copy link

boring-cyborg bot commented Jun 21, 2024

Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.

@Dawnpool Dawnpool changed the title Improve SFTPOperator with directory path and DELETE operation Improve SFTPOperator with directory transfer and DELETE operation Jun 21, 2024
@potiuk potiuk added good first issue and removed needs-triage label for new issues that we didn't triage yet labels Jun 21, 2024
@marcindulak
Copy link

I think this operator would be more powerful if it could transfer an entire folder by specifying just the folder name as well as delete files and folders with DELETE method.

What are people doing currently to recursively obtain the list of all files in a remote directory on sftp server?

Would adding the MSLD command, to list the remote directory also be useful?

@Dawnpool
Copy link
Contributor Author

Hi @marcindulak
I am not sure what most people do to recursively obtain the list of all files in a directory, but I was considering writing a recursive function using depth-first search for that in this operator code.
As you mentioned, adding the MSLD command would be very useful. I found that the describe_directory function in SFTPHook actually uses the MLSD command.
However, from my research, only FTP supports MLSD, not SFTP, so, I am not sure if I would be able to use it.
Thank you for your opinion, and please let me know if you have any further suggestions.

@Dawnpool
Copy link
Contributor Author

Hi guys @potiuk @vatsrahul1001 @shahar1 @eladkal
I am mentioning all of you because I am not sure who is mainly in charge of this feature.
Can I create a PR for this one?

@shahar1
Copy link
Contributor

shahar1 commented Jul 15, 2024

Hi guys @potiuk @vatsrahul1001 @shahar1 @eladkal
I am mentioning all of you because I am not sure who is mainly in charge of this feature.
Can I create a PR for this one?

Feel free to create a PR - I'll be happy to review.

@potiuk
Copy link
Member

potiuk commented Jul 15, 2024

FYI @Dawnpool - it does not work like this. No-one is "in charge". Just create a PR and whoever will be available (and have time and feels like it's OK to do it will (at some point in time) review the PR. Just raise a PR and gently remind (in general) that it needs a review, rather than pinging individual people.

@Dawnpool
Copy link
Contributor Author

FYI @Dawnpool - it does not work like this. No-one is "in charge". Just create a PR and whoever will be available (and have time and feels like it's OK to do it will (at some point in time) review the PR. Just raise a PR and gently remind (in general) that it needs a review, rather than pinging individual people.

Alright! I will keep that in mind. I will create a PR soon. Thanks for letting me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants