Lee-W commented on code in PR #52700:
URL: https://github.com/apache/airflow/pull/52700#discussion_r2193843889
##########
providers/sftp/src/airflow/providers/sftp/hooks/sftp.py:
##########
@@ -130,6 +135,23 @@ def close_conn(self) -> None:
self.conn.close()
self.conn = None
+ def _throw_if_none(self, to_check, exception: Exception):
Review Comment:
```suggestion
def _throw_if_none(self, to_check, exception: Exception) -> None:
```
##########
providers/sftp/src/airflow/providers/sftp/hooks/sftp.py:
##########
@@ -72,6 +72,9 @@ class SFTPHook(SSHHook):
default_conn_name = "sftp_default"
conn_type = "sftp"
hook_name = "SFTP"
+ CONNECTION_NOT_OPEN_EXCEPTION: AirflowException = AirflowException(
Review Comment:
We should create a customized exception here instead of using
AirflowExepciton
https://github.com/apache/airflow/blob/main/providers/standard/src/airflow/providers/standard/exceptions.py
##########
providers/sftp/src/airflow/providers/sftp/hooks/sftp.py:
##########
@@ -130,6 +135,23 @@ def close_conn(self) -> None:
self.conn.close()
self.conn = None
+ def _throw_if_none(self, to_check, exception: Exception):
Review Comment:
not sure whether we need this method, it's not enhancing readability IMO
##########
providers/sftp/src/airflow/providers/sftp/hooks/sftp.py:
##########
@@ -155,6 +177,18 @@ def get_conn_count(self) -> int:
"""Get the number of open connections."""
return self._conn_count
+ def _describe_directory(self, conn: SFTPClient, path: str) -> dict[str,
dict[str, str | int | None]]:
+ flist = sorted(conn.listdir_attr(path), key=lambda x: x.filename)
+ files = {}
+ for f in flist:
+ modify =
datetime.datetime.fromtimestamp(f.st_mtime).strftime("%Y%m%d%H%M%S") # type:
ignore
+ files[f.filename] = {
+ "size": f.st_size,
+ "type": "dir" if stat.S_ISDIR(f.st_mode) else "file", # type:
ignore
+ "modify": modify,
+ }
+ return files
Review Comment:
```suggestion
return {
f.filename: {
"size": f.st_size,
"type": "dir" if stat.S_ISDIR(f.st_mode) else "file", #
type: ignore
"modify":
datetime.datetime.fromtimestamp(f.st_mtime).strftime("%Y%m%d%H%M%S"),
}
for f in sorted(conn.listdir_attr(path), key=lambda x:
x.filename)
}
```
also we could make the return value a TypedDict
##########
providers/sftp/src/airflow/providers/sftp/hooks/sftp.py:
##########
@@ -130,6 +135,23 @@ def close_conn(self) -> None:
self.conn.close()
self.conn = None
+ def _throw_if_none(self, to_check, exception: Exception):
+ if to_check is None:
+ raise exception
+
+ def _handle_not_managed_conn(self, run_if_success: Callable):
Review Comment:
```suggestion
def _handle_not_managed_conn(self, run_if_success: Callable) ->
tuple[None, bool]:
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
