potiuk commented on a change in pull request #15408:
URL: https://github.com/apache/airflow/pull/15408#discussion_r619688934
##########
File path: airflow/providers/sftp/hooks/sftp.py
##########
@@ -45,9 +45,12 @@ class SFTPHook(SSHHook):
permissions.
Errors that may occur throughout but should be handled downstream.
+
+ :param sftp_conn_id: The :ref:`sftp connection id<howto/connection:sftp>`
+ :type sftp_conn_id: str
"""
- conn_name_attr = 'ftp_conn_id'
+ conn_name_attr = 'sftp_conn_id'
Review comment:
This change is wrong.
It should remain ftp_conn_id because this is the name of the constructor
argument (below:
```
def __init__(self, ftp_conn_id: str = 'sftp_default', *args, **kwargs)
-> None:
```
It is later remapped to "ssh_conn_id" just before running the constructor of
the parent class, so it does not matter. Note that it is just a name used when
the connection is instantiated dynamically so it does not really matter what it
is is, as long as it is consistent. And if someone ever created the Hook
manually with keyword argument, changing it would mean backwards
incompatibility as mentioned by @uranusjr
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
