From ab794d780b1c068788e61e6cf0799cf6f8310397 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mose=20M=C3=BCller?= Date: Mon, 27 Nov 2023 17:16:15 +0100 Subject: [PATCH] implements logging suggestions (no f-strings) --- src/pydase/components/number_slider.py | 4 +-- src/pydase/data_service/callback_manager.py | 2 +- src/pydase/data_service/data_service.py | 19 ++++++++----- src/pydase/data_service/state_manager.py | 30 +++++++++++---------- src/pydase/data_service/task_manager.py | 12 +++++---- src/pydase/server/server.py | 24 +++++++++-------- src/pydase/server/web_server.py | 4 +-- src/pydase/utils/helpers.py | 6 ++--- src/pydase/utils/warnings.py | 3 ++- tests/data_service/test_state_manager.py | 2 +- tests/utils/test_warnings.py | 2 +- 11 files changed, 60 insertions(+), 48 deletions(-) diff --git a/src/pydase/components/number_slider.py b/src/pydase/components/number_slider.py index 7a2ac9e..4610be9 100644 --- a/src/pydase/components/number_slider.py +++ b/src/pydase/components/number_slider.py @@ -44,10 +44,10 @@ class NumberSlider(DataService): min: float = 0.0, max: float = 100.0, step_size: float | int = 1.0, - type: Literal["int"] | Literal["float"] = "float", + type: Literal["int", "float"] = "float", ) -> None: if type not in {"float", "int"}: - logger.error(f"Unknown type '{type}'. Using 'float'.") + logger.error("Unknown type '%s'. Using 'float'.", type) type = "float" self._type = type diff --git a/src/pydase/data_service/callback_manager.py b/src/pydase/data_service/callback_manager.py index 29da9f0..2b868c8 100644 --- a/src/pydase/data_service/callback_manager.py +++ b/src/pydase/data_service/callback_manager.py @@ -411,7 +411,7 @@ class CallbackManager: ) def emit_notification(self, parent_path: str, name: str, value: Any) -> None: - logger.debug(f"{parent_path}.{name} changed to {value}!") + logger.debug("%s.%s changed to %s!", parent_path, name, value) for callback in self._notification_callbacks: try: diff --git a/src/pydase/data_service/data_service.py b/src/pydase/data_service/data_service.py index 445ac14..0a961d1 100644 --- a/src/pydase/data_service/data_service.py +++ b/src/pydase/data_service/data_service.py @@ -85,9 +85,10 @@ class DataService(rpyc.Service, AbstractDataService): callback(__name, __value) elif __name.startswith(f"_{self.__class__.__name__}__"): logger.warning( - f"Warning: You should not set private but rather protected attributes! " - f"Use {__name.replace(f'_{self.__class__.__name__}__', '_')} instead " - f"of {__name.replace(f'_{self.__class__.__name__}__', '__')}." + "Warning: You should not set private but rather protected attributes! " + "Use %s instead of %s.", + __name.replace(f"_{self.__class__.__name__}__", "_"), + __name.replace(f"_{self.__class__.__name__}__", "__"), ) def __check_instance_classes(self) -> None: @@ -178,8 +179,9 @@ class DataService(rpyc.Service, AbstractDataService): class_attr_is_read_only = nested_class_dict["readonly"] if class_attr_is_read_only: logger.debug( - f'Attribute "{path}" is read-only. Ignoring value from JSON ' - "file..." + "Attribute '%s' is read-only. Ignoring value from JSON " + "file...", + path, ) continue # Split the path into parts @@ -193,8 +195,11 @@ class DataService(rpyc.Service, AbstractDataService): self.update_DataService_attribute(parts[:-1], attr_name, value) else: logger.info( - f'Attribute type of "{path}" changed from "{value_type}" to ' - f'"{class_value_type}". Ignoring value from JSON file...' + "Attribute type of '%s' changed from '%s' to " + "'%s'. Ignoring value from JSON file...", + path, + value_type, + class_value_type, ) def serialize(self) -> dict[str, dict[str, Any]]: # noqa diff --git a/src/pydase/data_service/state_manager.py b/src/pydase/data_service/state_manager.py index c740eea..3ffaa03 100644 --- a/src/pydase/data_service/state_manager.py +++ b/src/pydase/data_service/state_manager.py @@ -102,7 +102,7 @@ class StateManager: if filename is not None: if self.filename is not None: logger.warning( - f"Overwriting filename {self.filename!r} with {filename!r}." + "Overwriting filename '%s' with '%s'.", self.filename, filename ) self.filename = filename @@ -155,18 +155,19 @@ class StateManager: self.set_service_attribute_value_by_path(path, value) else: logger.info( - f"Attribute type of {path!r} changed from {value_type!r} to " - f"{class_attr_value_type!r}. Ignoring value from JSON file..." + "Attribute type of '%s' changed from '%s' to " + "'%s'. Ignoring value from JSON file...", + path, + value_type, + class_attr_value_type, ) def _get_state_dict_from_JSON_file(self) -> dict[str, Any]: - if self.filename is not None: - # Check if the file specified by the filename exists - if os.path.exists(self.filename): - with open(self.filename, "r") as f: - # Load JSON data from file and update class attributes with these - # values - return cast(dict[str, Any], json.load(f)) + if self.filename is not None and os.path.exists(self.filename): + with open(self.filename, "r") as f: + # Load JSON data from file and update class attributes with these + # values + return cast(dict[str, Any], json.load(f)) return {} def set_service_attribute_value_by_path( @@ -192,7 +193,7 @@ class StateManager: # This will also filter out methods as they are 'read-only' if current_value_dict["readonly"]: - logger.debug(f"Attribute {path!r} is read-only. Ignoring new value...") + logger.debug("Attribute '%s' is read-only. Ignoring new value...", path) return converted_value = self.__convert_value_if_needed(value, current_value_dict) @@ -201,7 +202,7 @@ class StateManager: if self.__attr_value_has_changed(converted_value, current_value_dict["value"]): self.__update_attribute_by_path(path, converted_value) else: - logger.debug(f"Value of attribute {path!r} has not changed...") + logger.debug("Value of attribute '%s' has not changed...", path) def __attr_value_has_changed(self, value_object: Any, current_value: Any) -> bool: """Check if the serialized value of `value_object` differs from `current_value`. @@ -262,8 +263,9 @@ class StateManager: has_decorator = has_load_state_decorator(prop) if not has_decorator: logger.debug( - f"Property {attr_name!r} has no '@load_state' decorator. " - "Ignoring value from JSON file..." + "Property '%s' has no '@load_state' decorator. " + "Ignoring value from JSON file...", + attr_name, ) return has_decorator return True diff --git a/src/pydase/data_service/task_manager.py b/src/pydase/data_service/task_manager.py index dd50be5..e7f0b35 100644 --- a/src/pydase/data_service/task_manager.py +++ b/src/pydase/data_service/task_manager.py @@ -111,7 +111,7 @@ class TaskManager: start_method(*args) else: logger.warning( - f"No start method found for service '{service_name}'" + "No start method found for service '%s'", service_name ) def start_autostart_tasks(self) -> None: @@ -179,8 +179,10 @@ class TaskManager: if exception is not None: # Handle the exception, or you can re-raise it. logger.error( - f"Task '{name}' encountered an exception: " - f"{type(exception).__name__}: {exception}" + "Task '%s' encountered an exception: %s: %s", + name, + type(exception).__name__, + exception, ) raise exception @@ -188,7 +190,7 @@ class TaskManager: try: await method(*args, **kwargs) except asyncio.CancelledError: - logger.info(f"Task {name} was cancelled") + logger.info("Task '%s' was cancelled", name) if not self.tasks.get(name): # Get the signature of the coroutine method to start @@ -230,6 +232,6 @@ class TaskManager: for callback in self.task_status_change_callbacks: callback(name, kwargs_updated) else: - logger.error(f"Task `{name}` is already running!") + logger.error("Task '%s' is already running!", name) return start_task diff --git a/src/pydase/server/server.py b/src/pydase/server/server.py index ba0d900..d247be3 100644 --- a/src/pydase/server/server.py +++ b/src/pydase/server/server.py @@ -234,7 +234,7 @@ class Server: async def serve(self) -> None: process_id = os.getpid() - logger.info(f"Started server process [{process_id}]") + logger.info("Started server process [%s]", process_id) await self.startup() if self.should_exit: @@ -242,7 +242,7 @@ class Server: await self.main_loop() await self.shutdown() - logger.info(f"Finished server process [{process_id}]") + logger.info("Finished server process [%s]", process_id) async def startup(self) -> None: # noqa: C901 self._loop = asyncio.get_running_loop() @@ -330,7 +330,7 @@ class Server: }, ) except Exception as e: - logger.warning(f"Failed to send notification: {e}") + logger.warning("Failed to send notification: %s", e) self._loop.create_task(notify()) @@ -349,7 +349,7 @@ class Server: async def shutdown(self) -> None: logger.info("Shutting down") - logger.info(f"Saving data to {self._state_manager.filename}.") + logger.info("Saving data to %s.", self._state_manager.filename) if self._state_manager is not None: self._state_manager.save_state() @@ -366,9 +366,9 @@ class Server: try: await task except asyncio.CancelledError: - logger.debug(f"Cancelled {server_name} server.") + logger.debug("Cancelled '%s' server.", server_name) except Exception as e: - logger.warning(f"Unexpected exception: {e}.") + logger.warning("Unexpected exception: %s", e) async def __cancel_tasks(self) -> None: for task in asyncio.all_tasks(self._loop): @@ -376,9 +376,9 @@ class Server: try: await task except asyncio.CancelledError: - logger.debug(f"Cancelled task {task.get_coro()}.") + logger.debug("Cancelled task '%s'.", task.get_coro()) except Exception as e: - logger.warning(f"Unexpected exception: {e}.") + logger.exception("Unexpected exception: %s", e) def install_signal_handlers(self) -> None: if threading.current_thread() is not threading.main_thread(): @@ -390,11 +390,13 @@ class Server: def handle_exit(self, sig: int = 0, frame: Optional[FrameType] = None) -> None: if self.should_exit and sig == signal.SIGINT: - logger.warning(f"Received signal {sig}, forcing exit...") + logger.warning("Received signal '%s', forcing exit...", sig) os._exit(1) else: self.should_exit = True - logger.warning(f"Received signal {sig}, exiting... (CTRL+C to force quit)") + logger.warning( + "Received signal '%s', exiting... (CTRL+C to force quit)", sig + ) def custom_exception_handler( self, loop: asyncio.AbstractEventLoop, context: dict[str, Any] @@ -421,7 +423,7 @@ class Server: }, ) except Exception as e: - logger.warning(f"Failed to send notification: {e}") + logger.exception("Failed to send notification: %s", e) loop.create_task(emit_exception()) else: diff --git a/src/pydase/server/web_server.py b/src/pydase/server/web_server.py index 47f79ae..6ab5a93 100644 --- a/src/pydase/server/web_server.py +++ b/src/pydase/server/web_server.py @@ -107,7 +107,7 @@ class WebAPI: @sio.event # type: ignore def set_attribute(sid: str, data: UpdateDict) -> Any: - logger.debug(f"Received frontend update: {data}") + logger.debug("Received frontend update: %s", data) path_list = [*data["parent_path"].split("."), data["name"]] path_list.remove("DataService") # always at the start, does not do anything path = ".".join(path_list) @@ -117,7 +117,7 @@ class WebAPI: @sio.event # type: ignore def run_method(sid: str, data: RunMethodDict) -> Any: - logger.debug(f"Running method: {data}") + logger.debug("Running method: %s", data) path_list = [*data["parent_path"].split("."), data["name"]] path_list.remove("DataService") # always at the start, does not do anything method = get_object_attr_from_path_list(self.service, path_list) diff --git a/src/pydase/utils/helpers.py b/src/pydase/utils/helpers.py index 3624b7a..35db422 100644 --- a/src/pydase/utils/helpers.py +++ b/src/pydase/utils/helpers.py @@ -59,7 +59,7 @@ def get_object_attr_from_path_list(target_obj: Any, path: list[str]) -> Any: target_obj = getattr(target_obj, part) except AttributeError: # The attribute doesn't exist - logger.debug(f"Attribute {part} does not exist in the object.") + logger.debug("Attribute % does not exist in the object.", part) return None return target_obj @@ -141,7 +141,7 @@ def update_value_if_changed( if getattr(target, attr_name_or_index) != new_value: setattr(target, attr_name_or_index, new_value) else: - logger.error(f"Incompatible arguments: {target}, {attr_name_or_index}.") + logger.error("Incompatible arguments: %s, %s.", target, attr_name_or_index) def parse_list_attr_and_index(attr_string: str) -> tuple[str, Optional[int]]: @@ -175,7 +175,7 @@ def parse_list_attr_and_index(attr_string: str) -> tuple[str, Optional[int]]: if index_part.isdigit(): index = int(index_part) else: - logger.error(f"Invalid index format in key: {attr_name}") + logger.error("Invalid index format in key: %s", attr_name) return attr_name, index diff --git a/src/pydase/utils/warnings.py b/src/pydase/utils/warnings.py index 0fe7eef..f77383c 100644 --- a/src/pydase/utils/warnings.py +++ b/src/pydase/utils/warnings.py @@ -22,5 +22,6 @@ def warn_if_instance_class_does_not_inherit_from_DataService(__value: object) -> and type(__value).__name__ not in ["CallbackManager", "TaskManager", "Quantity"] ): logger.warning( - f"Warning: Class {type(__value).__name__} does not inherit from DataService." + "Warning: Class '%s' does not inherit from DataService.", + type(__value).__name__, ) diff --git a/tests/data_service/test_state_manager.py b/tests/data_service/test_state_manager.py index f572e47..45827f0 100644 --- a/tests/data_service/test_state_manager.py +++ b/tests/data_service/test_state_manager.py @@ -162,7 +162,7 @@ def test_load_state(tmp_path: Path, caplog: LogCaptureFixture): "Ignoring value from JSON file..." ) in caplog.text assert ( - "Attribute type of 'removed_attr' changed from 'str' to None. " + "Attribute type of 'removed_attr' changed from 'str' to 'None'. " "Ignoring value from JSON file..." in caplog.text ) assert "Value of attribute 'subservice.name' has not changed..." in caplog.text diff --git a/tests/utils/test_warnings.py b/tests/utils/test_warnings.py index e0e0f81..a054b04 100644 --- a/tests/utils/test_warnings.py +++ b/tests/utils/test_warnings.py @@ -15,7 +15,7 @@ def test_setattr_warnings(caplog: LogCaptureFixture) -> None: # noqa ServiceClass() - assert "Warning: Class SubClass does not inherit from DataService." in caplog.text + assert "Warning: Class 'SubClass' does not inherit from DataService." in caplog.text def test_private_attribute_warning(caplog: LogCaptureFixture) -> None: # noqa