Merge pull request #7717 from RasaHQ/event-broker-async-warning

fix EventBroker async warning

Author: rasabot

changelog/7717.bugfix.md

@@ -0,0 +1,2 @@
+Fix wrong warning `The method 'EventBroker.close' was changed to be asynchronous` when
+the `EventBroker.close` was actually asynchronous.

rasa/core/exporter.py

@@ -87,9 +87,9 @@ async def publish_events(self) -> int:
                 logger.exception(e)
                 raise PublishingError(current_timestamp)
 
-        if asyncio.iscoroutinefunction(self.event_broker.close):
+        if not asyncio.iscoroutinefunction(self.event_broker.close):
             rasa.shared.utils.io.raise_deprecation_warning(
-                f"The method '{EventBroker.__name__}.{EventBroker.close.__name__} was "
+                f"The method '{EventBroker.__name__}.{EventBroker.close.__name__}' was "
                 f"changed to be asynchronous. Please adapt your custom event broker "
                 f"accordingly. Support for synchronous implementations will be removed "
                 f"in Rasa Open Source 3.0.0."

rasa/core/run.py

@@ -307,9 +307,9 @@ async def close_resources(app: Sanic, _: AbstractEventLoop) -> None:
 
     event_broker = current_agent.tracker_store.event_broker
     if event_broker:
-        if asyncio.iscoroutinefunction(event_broker.close):
+        if not asyncio.iscoroutinefunction(event_broker.close):
             rasa.shared.utils.io.raise_deprecation_warning(
-                f"The method '{EventBroker.__name__}.{EventBroker.close.__name__} was "
+                f"The method '{EventBroker.__name__}.{EventBroker.close.__name__}' was "
                 f"changed to be asynchronous. Please adapt your custom event broker "
                 f"accordingly. Support for synchronous implementations will be removed "
                 f"in Rasa Open Source 3.0.0."

tests/core/test_exporter.py

@@ -288,5 +288,32 @@ async def test_publishing_error():
 
     # run the export function
     with pytest.raises(PublishingError):
-        # noinspection PyProtectedMember
+        await exporter.publish_events()
+
+
+async def test_closing_broker():
+    exporter = MockExporter(event_broker=SQLEventBroker())
+
+    # noinspection PyProtectedMember
+    exporter._fetch_events_within_time_range = Mock(return_value=[])
+
+    # run the export function
+    with pytest.warns(None) as warnings:
+        await exporter.publish_events()
+
+    assert len(warnings) == 0
+
+
+async def test_closing_broker_sync():
+    class TestBroker(SQLEventBroker):
+        def close(self) -> None:
+            pass
+
+    exporter = MockExporter(event_broker=TestBroker())
+
+    # noinspection PyProtectedMember
+    exporter._fetch_events_within_time_range = Mock(return_value=[])
+
+    # run the export function
+    with pytest.warns(FutureWarning):
         await exporter.publish_events()

tests/core/test_run.py

@@ -1,3 +1,5 @@
+from unittest.mock import Mock
+
 import pytest
 from typing import Text
 
@@ -7,6 +9,7 @@
 from asyncio import AbstractEventLoop
 from pathlib import Path
 from rasa.core import run, interpreter, policies
+from rasa.core.brokers.sql import SQLEventBroker
 from rasa.core.utils import AvailableEndpoints
 
 CREDENTIALS_FILE = "examples/moodbot/credentials.yml"
@@ -82,3 +85,27 @@ async def test_load_agent_on_start_with_bad_model_file(
     assert isinstance(agent.interpreter, rasa.shared.nlu.interpreter.RegexInterpreter)
     assert agent.policy_ensemble is None
     assert isinstance(agent.domain, rasa.shared.core.domain.Domain)
+
+
+async def test_close_resources(loop: AbstractEventLoop):
+    broker = SQLEventBroker()
+    app = Mock()
+    app.agent.tracker_store.event_broker = broker
+
+    with pytest.warns(None) as warnings:
+        await run.close_resources(app, loop)
+
+    assert len(warnings) == 0
+
+
+async def test_close_resources_with_sync(loop: AbstractEventLoop):
+    class TestBroker(SQLEventBroker):
+        def close(self) -> None:
+            pass
+
+    broker = TestBroker()
+    app = Mock()
+    app.agent.tracker_store.event_broker = broker
+
+    with pytest.warns(FutureWarning):
+        await run.close_resources(app, loop)