From c93a5838bdffba3de88826e194b53b5863137237 Mon Sep 17 00:00:00 2001 From: JarbasAi Date: Tue, 3 Oct 2023 19:35:34 +0100 Subject: [PATCH] refactor/simplify session remove message history, makes payloads huge and is not consumed by anything --- ovos_bus_client/client/client.py | 2 +- ovos_bus_client/session.py | 70 ++++---------------------------- test/unittests/test_message.py | 4 +- test/unittests/test_session.py | 13 ------ 4 files changed, 10 insertions(+), 79 deletions(-) diff --git a/ovos_bus_client/client/client.py b/ovos_bus_client/client/client.py index 3185674..df5872c 100644 --- a/ovos_bus_client/client/client.py +++ b/ovos_bus_client/client/client.py @@ -176,7 +176,7 @@ def emit(self, message: Message): sess = SessionManager.sessions.get(self.session_id) or \ SessionManager.default_session message.context["session"] = sess.serialize() - sess.update_history(message) + sess.touch() if not self.connected_event.wait(10): diff --git a/ovos_bus_client/session.py b/ovos_bus_client/session.py index 8ae41d3..c5e976c 100644 --- a/ovos_bus_client/session.py +++ b/ovos_bus_client/session.py @@ -268,8 +268,6 @@ def get_context(self, max_frames: int = None, class Session: def __init__(self, session_id: str = None, expiration_seconds: int = None, active_skills: List[List[Union[str, float]]] = None, - history: List[Tuple[Message, float]] = None, - max_time: int = 5, max_messages: int = 5, utterance_states: dict = None, lang: str = None, context: IntentContextManager = None, valid_langs: List[str] = None, @@ -280,9 +278,6 @@ def __init__(self, session_id: str = None, expiration_seconds: int = None, @param session_id: string UUID for the session @param expiration_seconds: TTL for session (-1 for no expiration) @param active_skills: List of list skill_id, last reference - @param history: List of tuple Message, timestamp - @param max_time: maximum time for history in minutes - @param max_messages: maximum number of messages to retain @param utterance_states: dict of skill_id to UtteranceState @param lang: language associated with this Session @param context: IntentContextManager for this Session @@ -293,12 +288,9 @@ def __init__(self, session_id: str = None, expiration_seconds: int = None, self.site_id = site_id or "unknown" # indoors placement info self.valid_languages = valid_langs or _get_valid_langs() - self.active_skills = active_skills or [] # [skill_id , timestamp] - # TODO: Can active_skills be a list of tuples like history? - self.history = history or [] # (Message , timestamp) + self.active_skills = active_skills or [] # [skill_id , timestamp]# (Message , timestamp) self.utterance_states = utterance_states or {} # {skill_id: UtteranceState} - self.max_time = max_time # minutes - self.max_messages = max_messages + self.touch_time = int(time.time()) self.expiration_seconds = expiration_seconds or \ Configuration().get('session', {}).get("ttl", -1) @@ -348,7 +340,7 @@ def enable_response_mode(self, skill_id: str): @param skill_id: ID of skill expecting a response """ self.utterance_states[skill_id] = UtteranceState.RESPONSE.value - SessionManager.update(self) + self.touch() def disable_response_mode(self, skill_id: str): """ @@ -356,7 +348,7 @@ def disable_response_mode(self, skill_id: str): @param skill_id: ID of skill expecting a response """ self.utterance_states[skill_id] = UtteranceState.INTENT.value - SessionManager.update(self) + self.touch() def activate_skill(self, skill_id: str): """ @@ -367,7 +359,7 @@ def activate_skill(self, skill_id: str): self.deactivate_skill(skill_id) # add skill with timestamp to start of active list self.active_skills.insert(0, [skill_id, time.time()]) - SessionManager.update(self) + self.touch() def deactivate_skill(self, skill_id: str): """ @@ -378,7 +370,7 @@ def deactivate_skill(self, skill_id: str): if skill_id in active_ids: idx = active_ids.index(skill_id) self.active_skills.pop(idx) - SessionManager.update(self) + self.touch() def is_active(self, skill_id: str) -> bool: """ @@ -386,29 +378,15 @@ def is_active(self, skill_id: str) -> bool: @param skill_id: ID of skill to check @return: True if the requested skill is active """ - self._prune_history() # TODO: Is this necessary or useful? active_ids = [s[0] for s in self.active_skills] return skill_id in active_ids - def _prune_history(self): - """ - Remove messages from history that are too old or exceed max_messages - """ - # filter old messages from history - now = time.time() - self.history = [m for m in self.history - if now - m[1] < 60 * self.max_time] - # keep only self.max_messages - if len(self.history) > self.max_messages: - self.history = self.history[self.max_messages * -1:] - def clear(self): """ - Clear history and active_skills + Clear active_skills """ self.active_skills = [] - self.history = [] - SessionManager.update(self) + self.touch() def serialize(self) -> dict: """ @@ -419,7 +397,6 @@ def serialize(self) -> dict: "active_skills": self.active_skills, "utterance_states": self.utterance_states, "session_id": self.session_id, - "history": self.history, "lang": self.lang, "valid_languages": self.valid_languages, "context": self.context.serialize(), @@ -427,29 +404,6 @@ def serialize(self) -> dict: "pipeline": self.pipeline } - def update_history(self, message: Message = None): - """ - Add a message to history and then prune history - @param message: Message to append to history - """ - message = message or dig_for_message() - if message: - try: - m = message.as_dict - except AttributeError: - log_deprecation("mycroft-bus-client has been deprecated, please" - " update your imports to use ovos-bus-client", - "0.0.4", - excluded_package_refs=["ovos_bus_client"]) - m = json.loads(message.serialize()) - except Exception as e: - LOG.exception(e) - m = json.loads(message.serialize()) - m["context"] = {} # clear personal data - self.history.append((m, time.time())) - self._prune_history() - SessionManager.update(self) - @staticmethod def deserialize(data: dict): """ @@ -459,9 +413,6 @@ def deserialize(data: dict): """ uid = data.get("session_id") active = data.get("active_skills") or [] - history = data.get("history") or [] - max_time = data.get("max_time") or 5 - max_messages = data.get("max_messages") or 5 states = data.get("utterance_states") or {} lang = data.get("lang") valid_langs = data.get("valid_languages") or _get_valid_langs() @@ -471,11 +422,8 @@ def deserialize(data: dict): return Session(uid, active_skills=active, utterance_states=states, - history=history, - max_time=max_time, lang=lang, valid_langs=valid_langs, - max_messages=max_messages, context=context, pipeline=pipeline, site_id=site_id) @@ -569,8 +517,6 @@ def update(sess: Session, make_default: bool = False): if make_default: sess.session_id = "default" LOG.debug(f"replacing default session with: {sess.serialize()}") - else: - LOG.debug(f"session updated: {sess.session_id}") if sess.session_id == "default": SessionManager.default_session = sess diff --git a/test/unittests/test_message.py b/test/unittests/test_message.py index 40eae8c..33d51be 100644 --- a/test/unittests/test_message.py +++ b/test/unittests/test_message.py @@ -39,9 +39,7 @@ def test_serialize_deserialize(self): def test_session_serialize_deserialize(self): """Assert that a serized message is recreated when deserialized.""" s = Session() - for i in range(3): - s.update_history(Message(f'test_type_{i}', - context={"session": s.serialize()})) + SessionManager.update(s, make_default=True) source = Message('test_type', diff --git a/test/unittests/test_session.py b/test/unittests/test_session.py index a8487e2..29f25c8 100644 --- a/test/unittests/test_session.py +++ b/test/unittests/test_session.py @@ -140,10 +140,7 @@ def test_init(self): self.assertIsInstance(session.lang, str) self.assertIsInstance(session.valid_languages, list) self.assertEqual(session.active_skills, list()) - self.assertEqual(session.history, list()) self.assertEqual(session.utterance_states, dict()) - self.assertIsInstance(session.max_time, int) - self.assertIsInstance(session.max_messages, int) self.assertIsInstance(session.touch_time, int) self.assertIsInstance(session.expiration_seconds, int) self.assertIsInstance(session.context, IntentContextManager) @@ -194,10 +191,6 @@ def test_is_active(self): # TODO pass - def test_prune_history(self): - # TODO - pass - def test_clear(self): # TODO pass @@ -225,9 +218,7 @@ def test_serialize_deserialize(self): self.assertIsInstance(test_session.lang, str) self.assertIsInstance(test_session.valid_languages, list) self.assertIsInstance(test_session.active_skills, list) - self.assertIsInstance(test_session.history, list) self.assertIsInstance(test_session.utterance_states, dict) - self.assertIsInstance(test_session.max_time, int) self.assertIsInstance(test_session.touch_time, int) self.assertIsInstance(test_session.expiration_seconds, int) self.assertIsInstance(test_session.context, IntentContextManager) @@ -235,10 +226,6 @@ def test_serialize_deserialize(self): self.assertIsInstance(serialized, dict) self.assertIsInstance(serialized['context'], dict) - def test_update_history(self): - # TODO - pass - def test_from_message(self): # TODO pass