Taragolis commented on code in PR #28953:
URL: https://github.com/apache/airflow/pull/28953#discussion_r1070631126
##########
tests/providers/telegram/hooks/test_telegram.py:
##########
@@ -72,28 +71,31 @@ def
test_should_raise_exception_if_conn_id_doesnt_contain_token(self):
assert "Missing token(password) in Telegram connection" ==
str(ctx.value)
+ @pytest.mark.asyncio
@mock.patch("airflow.providers.telegram.hooks.telegram.TelegramHook.get_conn")
- def test_should_raise_exception_if_chat_id_is_not_provided_anywhere(self,
mock_get_conn):
+ async def
test_should_raise_exception_if_chat_id_is_not_provided_anywhere(self,
mock_get_conn):
Review Comment:
This wouldn't work correctly in Python 3.7
##########
airflow/providers/telegram/hooks/telegram.py:
##########
@@ -118,15 +118,15 @@ def __get_chat_id(self, chat_id: str | None,
telegram_conn_id: str | None) -> st
stop=tenacity.stop_after_attempt(5),
wait=tenacity.wait_fixed(1),
)
- def send_message(self, api_params: dict) -> None:
+ async def send_message(self, api_params: dict) -> None:
Review Comment:
I don't think we need to make hook's methods async. There are no any
beneficial of asyncio here - most of the code of Airflow is sync, only Triggers
expect async methods and there is no Triggers exists for Telegram provider.
But users who use previously use hooks methods in their code (callbacks,
custom Operators) can not use this methods without changing their code.
Better just wrap async code into the:
`asyncio.get_event_loop().run_until_complete(self.connection.send_message(**kwargs))`
--
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]
