Skip to content

Commit

Permalink
Resolve PT012 in SFTP provider tests (#38518)
Browse files Browse the repository at this point in the history
Co-authored-by: Bowrna <mailbowrna@gmail.com>
  • Loading branch information
shahar1 and Bowrna committed Apr 3, 2024
1 parent 40c70e3 commit 17eaf8e
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 46 deletions.
2 changes: 1 addition & 1 deletion airflow/providers/sftp/triggers/sftp.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ async def run(self) -> AsyncIterator[TriggerEvent]:
# Break loop to avoid infinite retries on terminal failure
break

yield TriggerEvent({"status": "error", "message": exc})
yield TriggerEvent({"status": "error", "message": str(exc)})

def _get_async_hook(self) -> SFTPHookAsync:
return SFTPHookAsync(sftp_conn_id=self.sftp_conn_id)
6 changes: 0 additions & 6 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -375,12 +375,6 @@ combine-as-imports = true
"tests/providers/qdrant/operators/test_qdrant.py" = ["E402"]
"tests/providers/snowflake/operators/test_snowflake_sql.py" = ["E402"]

# All the test modules which do not follow PT012 yet
"tests/providers/sftp/hooks/test_sftp.py" = ["PT012"]
"tests/providers/sftp/operators/test_sftp.py" = ["PT012"]
"tests/providers/sftp/sensors/test_sftp.py" = ["PT012"]
"tests/providers/sftp/triggers/test_sftp.py" = ["PT012"]

[tool.ruff.lint.flake8-tidy-imports]
# Disallow all relative imports.
ban-relative-imports = "all"
Expand Down
13 changes: 6 additions & 7 deletions tests/providers/sftp/hooks/test_sftp.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,13 +404,12 @@ def test_deprecation_ftp_conn_id(self, mock_get_connection):

@mock.patch("airflow.providers.sftp.hooks.sftp.SFTPHook.get_connection")
def test_invalid_ssh_hook(self, mock_get_connection):
with pytest.raises(AirflowException, match="ssh_hook must be an instance of SSHHook"):
connection = Connection(conn_id="sftp_default", login="root", host="localhost")
mock_get_connection.return_value = connection
with pytest.warns(
AirflowProviderDeprecationWarning, match=r"Parameter `ssh_hook` is deprecated.*"
):
SFTPHook(ssh_hook="invalid_hook")
connection = Connection(conn_id="sftp_default", login="root", host="localhost")
mock_get_connection.return_value = connection
with pytest.raises(AirflowException, match="ssh_hook must be an instance of SSHHook"), pytest.warns(
AirflowProviderDeprecationWarning, match=r"Parameter `ssh_hook` is deprecated.*"
):
SFTPHook(ssh_hook="invalid_hook")

@mock.patch("airflow.providers.ssh.hooks.ssh.SSHHook.get_connection")
def test_valid_ssh_hook(self, mock_get_connection):
Expand Down
47 changes: 24 additions & 23 deletions tests/providers/sftp/operators/test_sftp.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,14 +310,14 @@ def test_file_transfer_with_intermediate_dir_error_get(self, dag_maker, create_r
def test_arg_checking(self):
dag = DAG(dag_id="unit_tests_sftp_op_arg_checking", default_args={"start_date": DEFAULT_DATE})
# Exception should be raised if neither ssh_hook nor ssh_conn_id is provided
task_0 = SFTPOperator(
task_id="test_sftp_0",
local_filepath=self.test_local_filepath,
remote_filepath=self.test_remote_filepath,
operation=SFTPOperation.PUT,
dag=dag,
)
with pytest.raises(AirflowException, match="Cannot operate without sftp_hook or ssh_conn_id."):
task_0 = SFTPOperator(
task_id="test_sftp_0",
local_filepath=self.test_local_filepath,
remote_filepath=self.test_remote_filepath,
operation=SFTPOperation.PUT,
dag=dag,
)
task_0.execute(None)

# if ssh_hook is invalid/not provided, use ssh_conn_id to create SSHHook
Expand Down Expand Up @@ -360,31 +360,32 @@ def test_arg_checking(self):
task_3.execute(None)
assert task_3.sftp_hook.ssh_conn_id == self.hook.ssh_conn_id

task_4 = SFTPOperator(
task_id="test_sftp_4",
local_filepath=self.test_local_filepath,
remote_filepath=self.test_remote_filepath,
operation="invalid_operation",
dag=dag,
)
# Exception should be raised if operation is invalid
with pytest.raises(TypeError, match="Unsupported operation value invalid_operation, "):
task_4 = SFTPOperator(
task_id="test_sftp_4",
local_filepath=self.test_local_filepath,
remote_filepath=self.test_remote_filepath,
operation="invalid_operation",
dag=dag,
)
task_4.execute(None)

task_5 = SFTPOperator(
task_id="test_sftp_5",
ssh_hook=self.hook,
sftp_hook=SFTPHook(),
local_filepath=self.test_local_filepath,
remote_filepath=self.test_remote_filepath,
operation=SFTPOperation.PUT,
dag=dag,
)

# Exception should be raised if both ssh_hook and sftp_hook are provided
with pytest.raises(
AirflowException,
match="Both `ssh_hook` and `sftp_hook` are defined. Please use only one of them.",
):
task_5 = SFTPOperator(
task_id="test_sftp_5",
ssh_hook=self.hook,
sftp_hook=SFTPHook(),
local_filepath=self.test_local_filepath,
remote_filepath=self.test_remote_filepath,
operation=SFTPOperation.PUT,
dag=dag,
)
task_5.execute(None)

task_6 = SFTPOperator(
Expand Down
1 change: 0 additions & 1 deletion tests/providers/sftp/sensors/test_sftp.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ def test_sftp_failure(self, sftp_hook_mock, soft_fail: bool, expected_exception)
context = {"ds": "1970-01-01"}
with pytest.raises(expected_exception):
sftp_sensor.poke(context)
sftp_hook_mock.return_value.get_mod_time.assert_called_once_with("/path/to/file/1970-01-01.txt")

def test_hook_not_created_during_init(self):
sftp_sensor = SFTPSensor(task_id="unit_test", path="/path/to/file/1970-01-01.txt")
Expand Down
12 changes: 4 additions & 8 deletions tests/providers/sftp/triggers/test_sftp.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,10 @@ async def test_sftp_trigger_run_trigger_failure_state(self, mock_get_files_by_pa
mock_get_files_by_pattern.side_effect = Exception("An unexpected exception")

trigger = SFTPTrigger(path="test/path/", sftp_conn_id="sftp_default", file_pattern="my_test_file")

expected_event = {"status": "failure", "message": "An unexpected exception"}

with pytest.raises(Exception):
generator = trigger.run()
actual_event = await generator.asend(None)

assert TriggerEvent(expected_event) == actual_event
expected_event = {"status": "error", "message": "An unexpected exception"}
generator = trigger.run()
actual_event = await generator.asend(None)
assert TriggerEvent(expected_event) == actual_event

@pytest.mark.asyncio
@mock.patch("airflow.providers.sftp.hooks.sftp.SFTPHookAsync.get_files_and_attrs_by_pattern")
Expand Down

0 comments on commit 17eaf8e

Please sign in to comment.