From 15b59f17145e3d49753024ca8f11e885632b59bf Mon Sep 17 00:00:00 2001 From: melindaloubser1 Date: Fri, 19 Mar 2021 17:17:47 +0100 Subject: [PATCH 1/7] add condition in sql/mongo tracker store in additional_events --- changelog/8258.bugfix.md | 1 + docs/docs/connectors/custom-connectors.mdx | 2 +- docs/docs/http-api.mdx | 2 +- docs/docs/migration-guide.mdx | 2 +- rasa/core/tracker_store.py | 14 ++++++++++---- 5 files changed, 14 insertions(+), 7 deletions(-) create mode 100644 changelog/8258.bugfix.md diff --git a/changelog/8258.bugfix.md b/changelog/8258.bugfix.md new file mode 100644 index 000000000000..6b7f1c569136 --- /dev/null +++ b/changelog/8258.bugfix.md @@ -0,0 +1 @@ +Fixed the bug that events from previous conversation sessions would be re-saved in the SQL or Mongo tracker stores when `retrieve_events_from_previous_conversation_sessions` was true. diff --git a/docs/docs/connectors/custom-connectors.mdx b/docs/docs/connectors/custom-connectors.mdx index ec03bde78da6..ed6f60c3a699 100644 --- a/docs/docs/connectors/custom-connectors.mdx +++ b/docs/docs/connectors/custom-connectors.mdx @@ -48,7 +48,7 @@ the host and port with the appropriate values from your running Rasa X or Rasa O ## The `blueprint` method The `blueprint` method -needs to create a [sanic blueprint](https://sanic.readthedocs.io/en/stable/sanic/blueprints.html#blueprints) +needs to create a [sanic blueprint](https://sanicframework.org/en/guide/best-practices/blueprints.html#overview) that can be attached to a sanic server. Your blueprint should have at least the two routes: `health` on the route `/`, and `receive` on the route `/webhook` (see example custom channel below). diff --git a/docs/docs/http-api.mdx b/docs/docs/http-api.mdx index c028ce90bcdf..c537da92927c 100644 --- a/docs/docs/http-api.mdx +++ b/docs/docs/http-api.mdx @@ -39,7 +39,7 @@ By default, the HTTP server runs as a single process. You can change the number of worker processes using the `SANIC_WORKERS` environment variable. It is recommended that you set the number of workers to the number of available CPU cores (check out the -[Sanic docs](https://sanic.readthedocs.io/en/stable/sanic/deploying.html#workers) +[Sanic docs](https://sanicframework.org/en/guide/deployment/running.html#workers) for more details). This will only work in combination with the `RedisLockStore` (see [Lock Stores](./lock-stores.mdx). diff --git a/docs/docs/migration-guide.mdx b/docs/docs/migration-guide.mdx index b1f98eabffc1..cfac39bd3ef2 100644 --- a/docs/docs/migration-guide.mdx +++ b/docs/docs/migration-guide.mdx @@ -1085,7 +1085,7 @@ model before trying to use it with this improved version. * The `--num_threads` parameter was removed from the `run` command. The server will always run single-threaded, but will now run asynchronously. If you want to make use of multiple processes, feel free to check out the [Sanic server - documentation](https://sanic.readthedocs.io/en/stable/sanic/deploying.html#running-via-gunicorn). + documentation](https://sanicframework.org/en/guide/deployment/running.html#gunicorn). * To avoid conflicts in script parameter names, connectors in the `run` command now need to be specified with `--connector`, as `-c` is no longer supported. The maximum history in the `rasa visualize` command needs to be diff --git a/rasa/core/tracker_store.py b/rasa/core/tracker_store.py index 8b861f49b236..9bdde3be1d1c 100644 --- a/rasa/core/tracker_store.py +++ b/rasa/core/tracker_store.py @@ -625,9 +625,12 @@ def _additional_events(self, tracker: DialogueStateTracker) -> Iterator: stored = self.conversations.find_one({"sender_id": tracker.sender_id}) or {} all_events = self._events_from_serialized_tracker(stored) - number_events_since_last_session = len( - self._events_since_last_session_start(all_events) - ) + if self.retrieve_events_from_previous_conversation_sessions: + number_events_since_last_session = len(all_events) + else: + number_events_since_last_session = len( + self._events_since_last_session_start(all_events) + ) return itertools.islice( tracker.events, number_events_since_last_session, len(tracker.events) @@ -1128,8 +1131,11 @@ def _additional_events( """Return events from the tracker which aren't currently stored.""" number_of_events_since_last_session = self._event_query( - session, tracker.sender_id, fetch_events_from_all_sessions=False + session, + tracker.sender_id, + fetch_events_from_all_sessions=self.retrieve_events_from_previous_conversation_sessions, ).count() + return itertools.islice( tracker.events, number_of_events_since_last_session, len(tracker.events) ) From c581cc173da4de2f1c93fb629b98ae366f7b1a73 Mon Sep 17 00:00:00 2001 From: melindaloubser1 Date: Mon, 22 Mar 2021 12:37:57 +0100 Subject: [PATCH 2/7] remove blank line --- rasa/core/tracker_store.py | 1 - 1 file changed, 1 deletion(-) diff --git a/rasa/core/tracker_store.py b/rasa/core/tracker_store.py index 9bdde3be1d1c..c367499cedc2 100644 --- a/rasa/core/tracker_store.py +++ b/rasa/core/tracker_store.py @@ -1129,7 +1129,6 @@ def _additional_events( self, session: "Session", tracker: DialogueStateTracker ) -> Iterator: """Return events from the tracker which aren't currently stored.""" - number_of_events_since_last_session = self._event_query( session, tracker.sender_id, From 1be867bcf3c98dc441acc217913220f155864080 Mon Sep 17 00:00:00 2001 From: melindaloubser1 Date: Tue, 23 Mar 2021 10:08:31 +0100 Subject: [PATCH 3/7] parametrize additional_events tests to check retrieve_events_from_previous_conversation_sessions=true also --- tests/core/conftest.py | 9 ++++++- tests/core/test_tracker_stores.py | 40 ++++++++++++++++++++++++------- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/tests/core/conftest.py b/tests/core/conftest.py index 83f918d36abd..3780b6b9e934 100644 --- a/tests/core/conftest.py +++ b/tests/core/conftest.py @@ -84,11 +84,18 @@ def __init__(self, *args, **kwargs): class MockedMongoTrackerStore(MongoTrackerStore): """In-memory mocked version of `MongoTrackerStore`.""" - def __init__(self, _domain: Domain): + def __init__( + self, + _domain: Domain, + retrieve_events_from_previous_conversation_sessions: bool = False, + ): from mongomock import MongoClient self.db = MongoClient().rasa self.collection = "conversations" + self.retrieve_events_from_previous_conversation_sessions = ( + retrieve_events_from_previous_conversation_sessions + ) # skipcq: PYL-E1003 # Skip `MongoTrackerStore` constructor to avoid that actual Mongo connection diff --git a/tests/core/test_tracker_stores.py b/tests/core/test_tracker_stores.py index a59d2aa0931c..2d37dd8c7d61 100644 --- a/tests/core/test_tracker_stores.py +++ b/tests/core/test_tracker_stores.py @@ -539,8 +539,14 @@ def _saved_tracker_with_multiple_session_starts( return tracker_store.retrieve(sender_id) -def test_mongo_additional_events(default_domain: Domain): - tracker_store = MockedMongoTrackerStore(default_domain) +@pytest.mark.parametrize( + "retrieve_events_from_previous_conversation_sessions", [True, False], +) +def test_mongo_additional_events(default_domain: Domain, retrieve_events_from_previous_conversation_sessions): + tracker_store = MockedMongoTrackerStore( + default_domain, + retrieve_events_from_previous_conversation_sessions=retrieve_events_from_previous_conversation_sessions, + ) events, tracker = create_tracker_with_partially_saved_events(tracker_store) # make sure only new events are returned @@ -548,9 +554,15 @@ def test_mongo_additional_events(default_domain: Domain): assert list(tracker_store._additional_events(tracker)) == events -def test_mongo_additional_events_with_session_start(default_domain: Domain): +@pytest.mark.parametrize( + "retrieve_events_from_previous_conversation_sessions", [True, False], +) +def test_mongo_additional_events_with_session_start(default_domain: Domain, retrieve_events_from_previous_conversation_sessions): sender = "test_mongo_additional_events_with_session_start" - tracker_store = MockedMongoTrackerStore(default_domain) + tracker_store = MockedMongoTrackerStore( + default_domain, + retrieve_events_from_previous_conversation_sessions=retrieve_events_from_previous_conversation_sessions, + ) tracker = _saved_tracker_with_multiple_session_starts(tracker_store, sender) tracker.update(UserUttered("hi2")) @@ -562,10 +574,16 @@ def test_mongo_additional_events_with_session_start(default_domain: Domain): assert isinstance(additional_events[0], UserUttered) +@pytest.mark.parametrize( + "retrieve_events_from_previous_conversation_sessions", [True, False], +) # we cannot parametrise over this and the previous test due to the different ways of # calling _additional_events() -def test_sql_additional_events(default_domain: Domain): - tracker_store = SQLTrackerStore(default_domain) +def test_sql_additional_events(default_domain: Domain, retrieve_events_from_previous_conversation_sessions): + tracker_store = SQLTrackerStore( + default_domain, + retrieve_events_from_previous_conversation_sessions=retrieve_events_from_previous_conversation_sessions, + ) additional_events, tracker = create_tracker_with_partially_saved_events( tracker_store ) @@ -579,9 +597,15 @@ def test_sql_additional_events(default_domain: Domain): ) -def test_sql_additional_events_with_session_start(default_domain: Domain): +@pytest.mark.parametrize( + "retrieve_events_from_previous_conversation_sessions", [True, False], +) +def test_sql_additional_events_with_session_start(default_domain: Domain, retrieve_events_from_previous_conversation_sessions): sender = "test_sql_additional_events_with_session_start" - tracker_store = SQLTrackerStore(default_domain) + tracker_store = SQLTrackerStore( + default_domain, + retrieve_events_from_previous_conversation_sessions=retrieve_events_from_previous_conversation_sessions, + ) tracker = _saved_tracker_with_multiple_session_starts(tracker_store, sender) tracker.update(UserUttered("hi2"), default_domain) From 223cf221c6e58b5c5716a74b8d3482739f397292 Mon Sep 17 00:00:00 2001 From: Melinda Loubser <32034278+melindaloubser1@users.noreply.github.com> Date: Tue, 23 Mar 2021 10:09:19 +0100 Subject: [PATCH 4/7] Update changelog/8258.bugfix.md --- changelog/8258.bugfix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/8258.bugfix.md b/changelog/8258.bugfix.md index 6b7f1c569136..b89331d11178 100644 --- a/changelog/8258.bugfix.md +++ b/changelog/8258.bugfix.md @@ -1 +1 @@ -Fixed the bug that events from previous conversation sessions would be re-saved in the SQL or Mongo tracker stores when `retrieve_events_from_previous_conversation_sessions` was true. +Fixed the bug that events from previous conversation sessions would be re-saved in the [`SQLTrackerStore`](https://rasa.com/docs/rasa/tracker-stores#sqltrackerstore) or [`MongoTrackerStore`](https://rasa.com/docs/rasa/tracker-stores#mongotrackerstore) when `retrieve_events_from_previous_conversation_sessions` was true. From 4dd77f12fb2a43d06d7ff80506ace42121b2fef1 Mon Sep 17 00:00:00 2001 From: melindaloubser1 Date: Tue, 23 Mar 2021 10:48:05 +0100 Subject: [PATCH 5/7] format --- tests/core/test_tracker_stores.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/core/test_tracker_stores.py b/tests/core/test_tracker_stores.py index 2d37dd8c7d61..4f8639a24421 100644 --- a/tests/core/test_tracker_stores.py +++ b/tests/core/test_tracker_stores.py @@ -542,7 +542,9 @@ def _saved_tracker_with_multiple_session_starts( @pytest.mark.parametrize( "retrieve_events_from_previous_conversation_sessions", [True, False], ) -def test_mongo_additional_events(default_domain: Domain, retrieve_events_from_previous_conversation_sessions): +def test_mongo_additional_events( + default_domain: Domain, retrieve_events_from_previous_conversation_sessions +): tracker_store = MockedMongoTrackerStore( default_domain, retrieve_events_from_previous_conversation_sessions=retrieve_events_from_previous_conversation_sessions, @@ -557,7 +559,9 @@ def test_mongo_additional_events(default_domain: Domain, retrieve_events_from_pr @pytest.mark.parametrize( "retrieve_events_from_previous_conversation_sessions", [True, False], ) -def test_mongo_additional_events_with_session_start(default_domain: Domain, retrieve_events_from_previous_conversation_sessions): +def test_mongo_additional_events_with_session_start( + default_domain: Domain, retrieve_events_from_previous_conversation_sessions +): sender = "test_mongo_additional_events_with_session_start" tracker_store = MockedMongoTrackerStore( default_domain, @@ -579,7 +583,9 @@ def test_mongo_additional_events_with_session_start(default_domain: Domain, retr ) # we cannot parametrise over this and the previous test due to the different ways of # calling _additional_events() -def test_sql_additional_events(default_domain: Domain, retrieve_events_from_previous_conversation_sessions): +def test_sql_additional_events( + default_domain: Domain, retrieve_events_from_previous_conversation_sessions +): tracker_store = SQLTrackerStore( default_domain, retrieve_events_from_previous_conversation_sessions=retrieve_events_from_previous_conversation_sessions, @@ -600,7 +606,9 @@ def test_sql_additional_events(default_domain: Domain, retrieve_events_from_prev @pytest.mark.parametrize( "retrieve_events_from_previous_conversation_sessions", [True, False], ) -def test_sql_additional_events_with_session_start(default_domain: Domain, retrieve_events_from_previous_conversation_sessions): +def test_sql_additional_events_with_session_start( + default_domain: Domain, retrieve_events_from_previous_conversation_sessions +): sender = "test_sql_additional_events_with_session_start" tracker_store = SQLTrackerStore( default_domain, From 3f685343520b30070b5d1c38887a61ef4ebffe5b Mon Sep 17 00:00:00 2001 From: Melinda Loubser <32034278+melindaloubser1@users.noreply.github.com> Date: Wed, 24 Mar 2021 10:21:02 +0100 Subject: [PATCH 6/7] Update tests/core/conftest.py Co-authored-by: Tobias Wochinger --- tests/core/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/conftest.py b/tests/core/conftest.py index 3780b6b9e934..85af2840cdaa 100644 --- a/tests/core/conftest.py +++ b/tests/core/conftest.py @@ -88,7 +88,7 @@ def __init__( self, _domain: Domain, retrieve_events_from_previous_conversation_sessions: bool = False, - ): + ) -> None: from mongomock import MongoClient self.db = MongoClient().rasa From cc6b633ac31d2913b4f102eab04865818ecc2f5f Mon Sep 17 00:00:00 2001 From: Melinda Loubser <32034278+melindaloubser1@users.noreply.github.com> Date: Wed, 24 Mar 2021 10:21:41 +0100 Subject: [PATCH 7/7] Update changelog/8258.bugfix.md Co-authored-by: Tobias Wochinger --- changelog/8258.bugfix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/8258.bugfix.md b/changelog/8258.bugfix.md index b89331d11178..7f5b0efd76d2 100644 --- a/changelog/8258.bugfix.md +++ b/changelog/8258.bugfix.md @@ -1 +1 @@ -Fixed the bug that events from previous conversation sessions would be re-saved in the [`SQLTrackerStore`](https://rasa.com/docs/rasa/tracker-stores#sqltrackerstore) or [`MongoTrackerStore`](https://rasa.com/docs/rasa/tracker-stores#mongotrackerstore) when `retrieve_events_from_previous_conversation_sessions` was true. +Fixed the bug that events from previous conversation sessions would be re-saved in the [`SQLTrackerStore`](tracker-stores.mdx#sqltrackerstore) or [`MongoTrackerStore`](tracker-stores.mdx#mongotrackerstore) when `retrieve_events_from_previous_conversation_sessions` was true.