From 6d786cd0f80a23aa0c89b61f7c3c60a11b0da2c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mose=20M=C3=BCller?= Date: Mon, 29 Jul 2024 11:33:37 +0200 Subject: [PATCH 1/6] removes unused SerializationValueError exception --- src/pydase/data_service/data_service_cache.py | 3 +-- src/pydase/utils/serialization/serializer.py | 10 +--------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/pydase/data_service/data_service_cache.py b/src/pydase/data_service/data_service_cache.py index 4c59da0..1134d66 100644 --- a/src/pydase/data_service/data_service_cache.py +++ b/src/pydase/data_service/data_service_cache.py @@ -3,7 +3,6 @@ from typing import TYPE_CHECKING, Any, cast from pydase.utils.serialization.serializer import ( SerializationPathError, - SerializationValueError, SerializedObject, get_nested_dict_by_path, set_nested_value_by_path, @@ -43,7 +42,7 @@ class DataServiceCache: cast(dict[str, SerializedObject], self._cache["value"]), full_access_path, ) - except (SerializationPathError, SerializationValueError, KeyError): + except (SerializationPathError, KeyError): return { "full_access_path": full_access_path, "value": None, diff --git a/src/pydase/utils/serialization/serializer.py b/src/pydase/utils/serialization/serializer.py index 261c5ce..ab3e8f7 100644 --- a/src/pydase/utils/serialization/serializer.py +++ b/src/pydase/utils/serialization/serializer.py @@ -51,10 +51,6 @@ class SerializationPathError(Exception): pass -class SerializationValueError(Exception): - pass - - class Serializer: @classmethod def serialize_object(cls, obj: Any, access_path: str = "") -> SerializedObject: # noqa: C901 @@ -358,7 +354,7 @@ def set_nested_value_by_path( next_level_serialized_object = get_container_item_by_key( current_dict, path_parts[-1], allow_append=True ) - except (SerializationPathError, SerializationValueError, KeyError) as e: + except (SerializationPathError, KeyError) as e: logger.error("Error occured trying to change %a: %s", path, e) return @@ -468,10 +464,6 @@ def get_container_item_by_key( If the path composed of `attr_name` and any specified index is invalid, or leads to an IndexError or KeyError. This error is also raised if an attempt to access a nonexistent key or index occurs without permission to append. - SerializationValueError: - If the retrieval results in an object that is expected to be a dictionary - but is not, indicating a mismatch between expected and actual serialized - data structure. """ processed_key = parse_serialized_key(key) From ad0f9420d95db8d755f22bd21cf30758053a0941 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mose=20M=C3=BCller?= Date: Mon, 29 Jul 2024 13:29:47 +0200 Subject: [PATCH 2/6] get_value_dict_from_cache does not catch exceptions any more --- src/pydase/data_service/data_service_cache.py | 18 +++---------- .../data_service/data_service_observer.py | 26 ++++++++++++++----- src/pydase/data_service/state_manager.py | 15 ++++++++--- 3 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/pydase/data_service/data_service_cache.py b/src/pydase/data_service/data_service_cache.py index 1134d66..66b94df 100644 --- a/src/pydase/data_service/data_service_cache.py +++ b/src/pydase/data_service/data_service_cache.py @@ -2,7 +2,6 @@ import logging from typing import TYPE_CHECKING, Any, cast from pydase.utils.serialization.serializer import ( - SerializationPathError, SerializedObject, get_nested_dict_by_path, set_nested_value_by_path, @@ -37,16 +36,7 @@ class DataServiceCache: ) def get_value_dict_from_cache(self, full_access_path: str) -> SerializedObject: - try: - return get_nested_dict_by_path( - cast(dict[str, SerializedObject], self._cache["value"]), - full_access_path, - ) - except (SerializationPathError, KeyError): - return { - "full_access_path": full_access_path, - "value": None, - "type": "None", - "doc": None, - "readonly": False, - } + return get_nested_dict_by_path( + cast(dict[str, SerializedObject], self._cache["value"]), + full_access_path, + ) diff --git a/src/pydase/data_service/data_service_observer.py b/src/pydase/data_service/data_service_observer.py index febd157..ca74dc6 100644 --- a/src/pydase/data_service/data_service_observer.py +++ b/src/pydase/data_service/data_service_observer.py @@ -9,7 +9,11 @@ from pydase.observer_pattern.observer.property_observer import ( PropertyObserver, ) from pydase.utils.helpers import get_object_attr_from_path -from pydase.utils.serialization.serializer import SerializedObject, dump +from pydase.utils.serialization.serializer import ( + SerializationPathError, + SerializedObject, + dump, +) logger = logging.getLogger(__name__) @@ -29,17 +33,27 @@ class DataServiceObserver(PropertyObserver): for changing_attribute in self.changing_attributes ): return + cached_value_dict: SerializedObject - cached_value_dict = deepcopy( - self.state_manager._data_service_cache.get_value_dict_from_cache( - full_access_path + try: + cached_value_dict = deepcopy( + self.state_manager._data_service_cache.get_value_dict_from_cache( + full_access_path + ) ) - ) + except (SerializationPathError, KeyError): + cached_value_dict = { + "full_access_path": full_access_path, + "value": None, + "type": "None", + "doc": None, + "readonly": False, + } cached_value = cached_value_dict.get("value") if ( all(part[0] != "_" for part in full_access_path.split(".")) - and cached_value != dump(value)["value"] + and cached_value != value ): logger.debug("'%s' changed to '%s'", full_access_path, value) diff --git a/src/pydase/data_service/state_manager.py b/src/pydase/data_service/state_manager.py index 98c58ed..5891b6d 100644 --- a/src/pydase/data_service/state_manager.py +++ b/src/pydase/data_service/state_manager.py @@ -157,9 +157,18 @@ class StateManager: for path in generate_serialized_data_paths(json_dict): if self.__is_loadable_state_attribute(path): nested_json_dict = get_nested_dict_by_path(json_dict, path) - nested_class_dict = self._data_service_cache.get_value_dict_from_cache( - path - ) + try: + nested_class_dict = ( + self._data_service_cache.get_value_dict_from_cache(path) + ) + except (SerializationPathError, KeyError): + nested_class_dict = { + "full_access_path": path, + "value": None, + "type": "None", + "doc": None, + "readonly": False, + } value_type = nested_json_dict["type"] class_attr_value_type = nested_class_dict.get("type", None) From 817afc610a998573dc61fa42e13ab29a589696d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mose=20M=C3=BCller?= Date: Mon, 29 Jul 2024 13:49:12 +0200 Subject: [PATCH 3/6] StateManager: replaces _data_service_cache with cache_manager - _data_service_cache -> cache_manager - removes cache property - replaces get_nested_dict_by_path with cache_manager.get_value_dict_from_cache where possible --- .../data_service/data_service_observer.py | 6 ++--- src/pydase/data_service/state_manager.py | 23 +++++++------------ src/pydase/server/web_server/sio_setup.py | 6 ++--- src/pydase/server/web_server/web_server.py | 2 +- tests/data_service/test_data_service_cache.py | 16 +++++-------- 5 files changed, 20 insertions(+), 33 deletions(-) diff --git a/src/pydase/data_service/data_service_observer.py b/src/pydase/data_service/data_service_observer.py index ca74dc6..958c930 100644 --- a/src/pydase/data_service/data_service_observer.py +++ b/src/pydase/data_service/data_service_observer.py @@ -37,7 +37,7 @@ class DataServiceObserver(PropertyObserver): try: cached_value_dict = deepcopy( - self.state_manager._data_service_cache.get_value_dict_from_cache( + self.state_manager.cache_manager.get_value_dict_from_cache( full_access_path ) ) @@ -60,7 +60,7 @@ class DataServiceObserver(PropertyObserver): self._update_cache_value(full_access_path, value, cached_value_dict) cached_value_dict = deepcopy( - self.state_manager._data_service_cache.get_value_dict_from_cache( + self.state_manager.cache_manager.get_value_dict_from_cache( full_access_path ) ) @@ -93,7 +93,7 @@ class DataServiceObserver(PropertyObserver): value_dict["type"], cached_value_dict["type"], ) - self.state_manager._data_service_cache.update_cache( + self.state_manager.cache_manager.update_cache( full_access_path, value, ) diff --git a/src/pydase/data_service/state_manager.py b/src/pydase/data_service/state_manager.py index 5891b6d..f7d69d7 100644 --- a/src/pydase/data_service/state_manager.py +++ b/src/pydase/data_service/state_manager.py @@ -113,19 +113,12 @@ class StateManager: self.filename = filename self.service = service - self._data_service_cache = DataServiceCache(self.service) - - @property - def cache(self) -> SerializedObject: - """Returns the cached DataService state.""" - return self._data_service_cache.cache + self.cache_manager = DataServiceCache(self.service) @property def cache_value(self) -> dict[str, SerializedObject]: """Returns the "value" value of the DataService serialization.""" - return cast( - dict[str, SerializedObject], self._data_service_cache.cache["value"] - ) + return cast(dict[str, SerializedObject], self.cache_manager.cache["value"]) def save_state(self) -> None: """ @@ -158,8 +151,8 @@ class StateManager: if self.__is_loadable_state_attribute(path): nested_json_dict = get_nested_dict_by_path(json_dict, path) try: - nested_class_dict = ( - self._data_service_cache.get_value_dict_from_cache(path) + nested_class_dict = self.cache_manager.get_value_dict_from_cache( + path ) except (SerializationPathError, KeyError): nested_class_dict = { @@ -211,7 +204,7 @@ class StateManager: value: The new value to set for the attribute. """ - current_value_dict = get_nested_dict_by_path(self.cache_value, path) + current_value_dict = self.cache_manager.get_value_dict_from_cache(path) # This will also filter out methods as they are 'read-only' if current_value_dict["readonly"]: @@ -249,7 +242,7 @@ class StateManager: path_parts = parse_full_access_path(path) target_obj = get_object_by_path_parts(self.service, path_parts[:-1]) - attr_cache_type = get_nested_dict_by_path(self.cache_value, path)["type"] + attr_cache_type = self.cache_manager.get_value_dict_from_cache(path)["type"] # De-serialize the value if attr_cache_type in ("ColouredEnum", "Enum"): @@ -296,8 +289,8 @@ class StateManager: return has_decorator try: - cached_serialization_dict = get_nested_dict_by_path( - self.cache_value, full_access_path + cached_serialization_dict = self.cache_manager.get_value_dict_from_cache( + full_access_path ) if cached_serialization_dict["value"] == "method": diff --git a/src/pydase/server/web_server/sio_setup.py b/src/pydase/server/web_server/sio_setup.py index 0cc4505..aa160f4 100644 --- a/src/pydase/server/web_server/sio_setup.py +++ b/src/pydase/server/web_server/sio_setup.py @@ -134,7 +134,7 @@ def setup_sio_events(sio: socketio.AsyncServer, state_manager: StateManager) -> "Client [%s] requested service serialization", click.style(str(sid), fg="cyan"), ) - return state_manager.cache + return state_manager.cache_manager.cache @sio.event async def update_value(sid: str, data: UpdateDict) -> SerializedObject | None: # type: ignore @@ -151,9 +151,7 @@ def setup_sio_events(sio: socketio.AsyncServer, state_manager: StateManager) -> @sio.event async def get_value(sid: str, access_path: str) -> SerializedObject: try: - return state_manager._data_service_cache.get_value_dict_from_cache( - access_path - ) + return state_manager.cache_manager.get_value_dict_from_cache(access_path) except Exception as e: logger.exception(e) return dump(e) diff --git a/src/pydase/server/web_server/web_server.py b/src/pydase/server/web_server/web_server.py index 3fc1084..1dd6c96 100644 --- a/src/pydase/server/web_server/web_server.py +++ b/src/pydase/server/web_server/web_server.py @@ -123,7 +123,7 @@ class WebServer: self, request: aiohttp.web.Request, ) -> aiohttp.web.Response: - return aiohttp.web.json_response(self.state_manager.cache) + return aiohttp.web.json_response(self.state_manager.cache_manager.cache) async def _web_settings_route( self, diff --git a/tests/data_service/test_data_service_cache.py b/tests/data_service/test_data_service_cache.py index 00f943d..8bb734a 100644 --- a/tests/data_service/test_data_service_cache.py +++ b/tests/data_service/test_data_service_cache.py @@ -22,13 +22,13 @@ def test_nested_attributes_cache_callback() -> None: service_instance.name = "Peepz" assert ( - state_manager._data_service_cache.get_value_dict_from_cache("name")["value"] + state_manager.cache_manager.get_value_dict_from_cache("name")["value"] == "Peepz" ) service_instance.class_attr.name = "Ciao" assert ( - state_manager._data_service_cache.get_value_dict_from_cache("class_attr.name")[ + state_manager.cache_manager.get_value_dict_from_cache("class_attr.name")[ "value" ] == "Ciao" @@ -48,24 +48,20 @@ async def test_task_status_update() -> None: DataServiceObserver(state_manager) assert ( - state_manager._data_service_cache.get_value_dict_from_cache("my_method")["type"] + state_manager.cache_manager.get_value_dict_from_cache("my_method")["type"] == "method" ) assert ( - state_manager._data_service_cache.get_value_dict_from_cache("my_method")[ - "value" - ] + state_manager.cache_manager.get_value_dict_from_cache("my_method")["value"] is None ) service_instance.start_my_method() # type: ignore assert ( - state_manager._data_service_cache.get_value_dict_from_cache("my_method")["type"] + state_manager.cache_manager.get_value_dict_from_cache("my_method")["type"] == "method" ) assert ( - state_manager._data_service_cache.get_value_dict_from_cache("my_method")[ - "value" - ] + state_manager.cache_manager.get_value_dict_from_cache("my_method")["value"] == "RUNNING" ) From d54eed8a587a8592c4cba36b9dfeb7a6176f4964 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mose=20M=C3=BCller?= Date: Mon, 29 Jul 2024 14:16:43 +0200 Subject: [PATCH 4/6] get_object_by_path_parts and get_object_attr_from_path do not catch exceptions any more --- src/pydase/utils/helpers.py | 19 +++++++++++++------ tests/utils/test_helpers.py | 18 +++++++++++++++--- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/pydase/utils/helpers.py b/src/pydase/utils/helpers.py index f5f83c2..7b685ab 100644 --- a/src/pydase/utils/helpers.py +++ b/src/pydase/utils/helpers.py @@ -122,16 +122,20 @@ def get_class_and_instance_attributes(obj: object) -> dict[str, Any]: def get_object_by_path_parts(target_obj: Any, path_parts: list[str]) -> Any: + """Gets nested attribute of `target_object` specified by `path_parts`. + + Raises: + AttributeError: Attribute does not exist. + KeyError: Key in dict does not exist. + IndexError: Index out of list range. + TypeError: List index in the path is not a valid integer. + """ for part in path_parts: if part.startswith("["): deserialized_part = parse_serialized_key(part) target_obj = target_obj[deserialized_part] else: - try: - target_obj = getattr(target_obj, part) - except AttributeError: - logger.debug("Attribute %a does not exist in the object.", part) - return None + target_obj = getattr(target_obj, part) return target_obj @@ -149,7 +153,10 @@ def get_object_attr_from_path(target_obj: Any, path: str) -> Any: the path does not exist, the function logs a debug message and returns None. Raises: - ValueError: If a list index in the path is not a valid integer. + AttributeError: Attribute does not exist. + KeyError: Key in dict does not exist. + IndexError: Index out of list range. + TypeError: List index in the path is not a valid integer. """ path_parts = parse_full_access_path(path) return get_object_by_path_parts(target_obj, path_parts) diff --git a/tests/utils/test_helpers.py b/tests/utils/test_helpers.py index 9fe9894..da7ff6e 100644 --- a/tests/utils/test_helpers.py +++ b/tests/utils/test_helpers.py @@ -103,9 +103,21 @@ def test_get_object_by_path_parts(path_parts: list[str], expected: Any) -> None: assert get_object_by_path_parts(service_instance, path_parts) == expected -def test_get_object_by_path_parts_error(caplog: pytest.LogCaptureFixture) -> None: - assert get_object_by_path_parts(service_instance, ["non_existent_attr"]) is None - assert "Attribute 'non_existent_attr' does not exist in the object." in caplog.text +@pytest.mark.parametrize( + "path_parts, expected_exception", + [ + (["non_existent_attr"], AttributeError), + (["dict_attr", '["non_existent_key"]'], KeyError), + (["list_attr", "[2]"], IndexError), + (["list_attr", "[1.0]"], TypeError), + (["list_attr", '["string_key"]'], TypeError), + ], +) +def test_get_object_by_path_parts_exception( + path_parts: list[str], expected_exception: type[Exception] +) -> None: + with pytest.raises(expected_exception): + get_object_by_path_parts(service_instance, path_parts) @pytest.mark.parametrize( From 36d3a7becc35cb0e271fa859ab9e019e1254c99d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mose=20M=C3=BCller?= Date: Mon, 29 Jul 2024 14:56:57 +0200 Subject: [PATCH 5/6] restructure StateManager to allow extending dictionaries through clients --- src/pydase/data_service/state_manager.py | 39 +++++++++++++++++------- tests/client/test_client.py | 3 ++ 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/src/pydase/data_service/state_manager.py b/src/pydase/data_service/state_manager.py index f7d69d7..d8e22d3 100644 --- a/src/pydase/data_service/state_manager.py +++ b/src/pydase/data_service/state_manager.py @@ -1,3 +1,4 @@ +import contextlib import json import logging import os @@ -204,7 +205,16 @@ class StateManager: value: The new value to set for the attribute. """ - current_value_dict = self.cache_manager.get_value_dict_from_cache(path) + try: + current_value_dict = self.cache_manager.get_value_dict_from_cache(path) + except (SerializationPathError, KeyError): + current_value_dict = { + "full_access_path": path, + "value": None, + "type": "None", + "doc": None, + "readonly": False, + } # This will also filter out methods as they are 'read-only' if current_value_dict["readonly"]: @@ -239,24 +249,31 @@ class StateManager: def __update_attribute_by_path( self, path: str, serialized_value: SerializedObject ) -> None: + is_value_set = False path_parts = parse_full_access_path(path) target_obj = get_object_by_path_parts(self.service, path_parts[:-1]) - attr_cache_type = self.cache_manager.get_value_dict_from_cache(path)["type"] + def cached_value_is_enum(path: str) -> bool: + try: + attr_cache_type = self.cache_manager.get_value_dict_from_cache(path)[ + "type" + ] - # De-serialize the value - if attr_cache_type in ("ColouredEnum", "Enum"): + return attr_cache_type in ("ColouredEnum", "Enum") + except Exception: + return False + + if cached_value_is_enum(path): enum_attr = get_object_by_path_parts(target_obj, [path_parts[-1]]) # take the value of the existing enum class if serialized_value["type"] in ("ColouredEnum", "Enum"): - try: + # This error will arise when setting an enum from another enum class. + # In this case, we resort to loading the enum and setting it directly. + with contextlib.suppress(KeyError): value = enum_attr.__class__[serialized_value["value"]] - except KeyError: - # This error will arise when setting an enum from another enum class - # In this case, we resort to loading the enum and setting it - # directly - value = loads(serialized_value) - else: + is_value_set = True + + if not is_value_set: value = loads(serialized_value) # set the value diff --git a/tests/client/test_client.py b/tests/client/test_client.py index afd34ae..4303ad6 100644 --- a/tests/client/test_client.py +++ b/tests/client/test_client.py @@ -121,6 +121,9 @@ def test_dict(pydase_client: pydase.Client) -> None: # pop will remove the dictionary entry on the server assert list(pydase_client.proxy.dict_attr.keys()) == ["foo"] + pydase_client.proxy.dict_attr["non_existent_key"] = "Hello" + assert pydase_client.proxy.dict_attr["non_existent_key"] == "Hello" + def test_tab_completion(pydase_client: pydase.Client) -> None: # Tab completion gets its suggestions from the __dir__ class method From d09675de6a139df779dcee7392ce6adfb5c63ec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mose=20M=C3=BCller?= Date: Mon, 29 Jul 2024 15:06:54 +0200 Subject: [PATCH 6/6] updates client test --- tests/client/test_client.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/client/test_client.py b/tests/client/test_client.py index 4303ad6..fa4ef98 100644 --- a/tests/client/test_client.py +++ b/tests/client/test_client.py @@ -130,6 +130,7 @@ def test_tab_completion(pydase_client: pydase.Client) -> None: assert all( x in pydase_client.proxy.__dir__() for x in [ + "dict_attr", "list_attr", "my_method", "my_property",