SFTPHook requires param 'ftp_conn_id' to be passed even for ssh connections

[saveriogzz]

Apache Airflow Provider(s)

sftp

Versions of Apache Airflow Providers

apache-airflow-providers-sftp==2.2.0

Apache Airflow version

2.2.2 (latest released)

Operating System

Ubuntu 20.04.3 LTS

Deployment

Docker-Compose

Deployment details

No response

What happened

When instantiating an SFTPHook in order to create a connection to an sftp server, I noticed that if I was passing the parameter ssh_conn_id, the hook was trying to connect to localhost, whereas if I am passing ftp_conn_id, I obtain the desired connection.

airflow/airflow/providers/sftp/hooks/sftp.py

Lines 67 to 69 in 35be8bd

	def __init__(self, ftp_conn_id: str = 'sftp_default', *args, **kwargs) -> None:
	kwargs['ssh_conn_id'] = ftp_conn_id
	super().__init__(*args, **kwargs)

What you expected to happen

I would expect to be able to instantiate an sftp conection using the parameter ssh_conn_id instead of ftp_conn_id.

How to reproduce

No response

Anything else

No response

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[potiuk]

I would expect to be able to instantiate an sftp conection using the parameter ssh_conn_id instead of ftp_conn_id.

Why?

[saveriogzz]

To be consistent with the SFTPOperator and not to mislead the type of connection to be used since there are both FTP and SFTP connections!

[potiuk]

OK. Works for me. Sftp is really ssh under-the-hood indeed. But you should make sure to keep deprecated previous option working too. Assigning you :D

[kaxil]

I would say rather than deprecating, allow both as it is quite common to use SSH for get and send files

[potiuk]

It's really about the "iniit" parameter name of Hook, so I am not sure if we should have two variants :). One should be blesssed.

[potiuk]

And there is disconnect which one is used between the Operator and Hook.

[kaxil]

Oh yes this is regarding the Hook, I am fine with that.

The SftpOperator uses the SSHHook anyway so all food 😅 - good

[potiuk]

Food is good 🥫

[potiuk]

:D

[kaxil]

[eladkal]

fixed by #20164