Re: [PR] Resolve `PT012` in `SFTP` provider tests [airflow]
potiuk merged PR #38518: URL: https://github.com/apache/airflow/pull/38518 -- 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: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Resolve `PT012` in `SFTP` provider tests [airflow]
potiuk commented on PR #38518: URL: https://github.com/apache/airflow/pull/38518#issuecomment-2035482917 臘 -- 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: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Resolve `PT012` in `SFTP` provider tests [airflow]
shahar1 commented on code in PR #38518:
URL: https://github.com/apache/airflow/pull/38518#discussion_r1550356533
##
tests/providers/sftp/triggers/test_sftp.py:
##
@@ -175,13 +175,10 @@ async def
test_sftp_trigger_run_trigger_failure_state(self, mock_get_files_by_pa
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)
+generator = trigger.run()
+actual_event = await generator.asend(None)
Review Comment:
Merged the solution for this from #38525
--
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: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Resolve `PT012` in `SFTP` provider tests [airflow]
potiuk commented on PR #38518: URL: https://github.com/apache/airflow/pull/38518#issuecomment-2032877859 Or we can merge #38525 instead - @eladkal ? -- 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: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Resolve `PT012` in `SFTP` provider tests [airflow]
eladkal commented on PR #38518: URL: https://github.com/apache/airflow/pull/38518#issuecomment-2025568339 Can you rebase and resolve conflicts? -- 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: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Resolve `PT012` in `SFTP` provider tests [airflow]
Taragolis commented on code in PR #38518:
URL: https://github.com/apache/airflow/pull/38518#discussion_r1540207045
##
tests/providers/sftp/hooks/test_sftp.py:
##
@@ -405,12 +405,11 @@ 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.*"
-):
Review Comment:
I would rather keep this one, you could move `pytest.warns` in top of
`pytest.raises`, or join them, e.g.
```python
with pytest.raises(
AirflowException, match="ssh_hook must be an instance of SSHHook"
), pytest.warns(
AirflowProviderDeprecationWarning, match=r"Parameter `ssh_hook` is
deprecated.*"
):
```
--
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: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Resolve `PT012` in `SFTP` provider tests [airflow]
Taragolis commented on code in PR #38518:
URL: https://github.com/apache/airflow/pull/38518#discussion_r1540205185
##
tests/providers/sftp/triggers/test_sftp.py:
##
@@ -175,13 +175,10 @@ async def
test_sftp_trigger_run_trigger_failure_state(self, mock_get_files_by_pa
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)
+generator = trigger.run()
+actual_event = await generator.asend(None)
Review Comment:
I guess this test "worked" because `pytest.raises(Exception)` catch
assertion error from `assert TriggerEvent(expected_event) == actual_event`
--
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: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Resolve `PT012` in `SFTP` provider tests [airflow]
uranusjr commented on code in PR #38518:
URL: https://github.com/apache/airflow/pull/38518#discussion_r1540186628
##
tests/providers/sftp/triggers/test_sftp.py:
##
@@ -175,13 +175,10 @@ async def
test_sftp_trigger_run_trigger_failure_state(self, mock_get_files_by_pa
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)
+generator = trigger.run()
+actual_event = await generator.asend(None)
Review Comment:
This change seems wrong?
--
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: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
