implements logging suggestions (no f-strings)

This commit is contained in:
Mose Müller 2023-11-27 17:16:15 +01:00
parent 617eed4d96
commit ab794d780b
11 changed files with 60 additions and 48 deletions

View File

@ -44,10 +44,10 @@ class NumberSlider(DataService):
min: float = 0.0, min: float = 0.0,
max: float = 100.0, max: float = 100.0,
step_size: float | int = 1.0, step_size: float | int = 1.0,
type: Literal["int"] | Literal["float"] = "float", type: Literal["int", "float"] = "float",
) -> None: ) -> None:
if type not in {"float", "int"}: if type not in {"float", "int"}:
logger.error(f"Unknown type '{type}'. Using 'float'.") logger.error("Unknown type '%s'. Using 'float'.", type)
type = "float" type = "float"
self._type = type self._type = type

View File

@ -411,7 +411,7 @@ class CallbackManager:
) )
def emit_notification(self, parent_path: str, name: str, value: Any) -> None: 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: for callback in self._notification_callbacks:
try: try:

View File

@ -85,9 +85,10 @@ class DataService(rpyc.Service, AbstractDataService):
callback(__name, __value) callback(__name, __value)
elif __name.startswith(f"_{self.__class__.__name__}__"): elif __name.startswith(f"_{self.__class__.__name__}__"):
logger.warning( logger.warning(
f"Warning: You should not set private but rather protected attributes! " "Warning: You should not set private but rather protected attributes! "
f"Use {__name.replace(f'_{self.__class__.__name__}__', '_')} instead " "Use %s instead of %s.",
f"of {__name.replace(f'_{self.__class__.__name__}__', '__')}." __name.replace(f"_{self.__class__.__name__}__", "_"),
__name.replace(f"_{self.__class__.__name__}__", "__"),
) )
def __check_instance_classes(self) -> None: def __check_instance_classes(self) -> None:
@ -178,8 +179,9 @@ class DataService(rpyc.Service, AbstractDataService):
class_attr_is_read_only = nested_class_dict["readonly"] class_attr_is_read_only = nested_class_dict["readonly"]
if class_attr_is_read_only: if class_attr_is_read_only:
logger.debug( logger.debug(
f'Attribute "{path}" is read-only. Ignoring value from JSON ' "Attribute '%s' is read-only. Ignoring value from JSON "
"file..." "file...",
path,
) )
continue continue
# Split the path into parts # Split the path into parts
@ -193,8 +195,11 @@ class DataService(rpyc.Service, AbstractDataService):
self.update_DataService_attribute(parts[:-1], attr_name, value) self.update_DataService_attribute(parts[:-1], attr_name, value)
else: else:
logger.info( logger.info(
f'Attribute type of "{path}" changed from "{value_type}" to ' "Attribute type of '%s' changed from '%s' to "
f'"{class_value_type}". Ignoring value from JSON file...' "'%s'. Ignoring value from JSON file...",
path,
value_type,
class_value_type,
) )
def serialize(self) -> dict[str, dict[str, Any]]: # noqa def serialize(self) -> dict[str, dict[str, Any]]: # noqa

View File

@ -102,7 +102,7 @@ class StateManager:
if filename is not None: if filename is not None:
if self.filename is not None: if self.filename is not None:
logger.warning( logger.warning(
f"Overwriting filename {self.filename!r} with {filename!r}." "Overwriting filename '%s' with '%s'.", self.filename, filename
) )
self.filename = filename self.filename = filename
@ -155,18 +155,19 @@ class StateManager:
self.set_service_attribute_value_by_path(path, value) self.set_service_attribute_value_by_path(path, value)
else: else:
logger.info( logger.info(
f"Attribute type of {path!r} changed from {value_type!r} to " "Attribute type of '%s' changed from '%s' to "
f"{class_attr_value_type!r}. Ignoring value from JSON file..." "'%s'. Ignoring value from JSON file...",
path,
value_type,
class_attr_value_type,
) )
def _get_state_dict_from_JSON_file(self) -> dict[str, Any]: def _get_state_dict_from_JSON_file(self) -> dict[str, Any]:
if self.filename is not None: if self.filename is not None and os.path.exists(self.filename):
# Check if the file specified by the filename exists with open(self.filename, "r") as f:
if os.path.exists(self.filename): # Load JSON data from file and update class attributes with these
with open(self.filename, "r") as f: # values
# Load JSON data from file and update class attributes with these return cast(dict[str, Any], json.load(f))
# values
return cast(dict[str, Any], json.load(f))
return {} return {}
def set_service_attribute_value_by_path( def set_service_attribute_value_by_path(
@ -192,7 +193,7 @@ class StateManager:
# This will also filter out methods as they are 'read-only' # This will also filter out methods as they are 'read-only'
if current_value_dict["readonly"]: 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 return
converted_value = self.__convert_value_if_needed(value, current_value_dict) 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"]): if self.__attr_value_has_changed(converted_value, current_value_dict["value"]):
self.__update_attribute_by_path(path, converted_value) self.__update_attribute_by_path(path, converted_value)
else: 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: 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`. """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) has_decorator = has_load_state_decorator(prop)
if not has_decorator: if not has_decorator:
logger.debug( logger.debug(
f"Property {attr_name!r} has no '@load_state' decorator. " "Property '%s' has no '@load_state' decorator. "
"Ignoring value from JSON file..." "Ignoring value from JSON file...",
attr_name,
) )
return has_decorator return has_decorator
return True return True

View File

@ -111,7 +111,7 @@ class TaskManager:
start_method(*args) start_method(*args)
else: else:
logger.warning( 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: def start_autostart_tasks(self) -> None:
@ -179,8 +179,10 @@ class TaskManager:
if exception is not None: if exception is not None:
# Handle the exception, or you can re-raise it. # Handle the exception, or you can re-raise it.
logger.error( logger.error(
f"Task '{name}' encountered an exception: " "Task '%s' encountered an exception: %s: %s",
f"{type(exception).__name__}: {exception}" name,
type(exception).__name__,
exception,
) )
raise exception raise exception
@ -188,7 +190,7 @@ class TaskManager:
try: try:
await method(*args, **kwargs) await method(*args, **kwargs)
except asyncio.CancelledError: except asyncio.CancelledError:
logger.info(f"Task {name} was cancelled") logger.info("Task '%s' was cancelled", name)
if not self.tasks.get(name): if not self.tasks.get(name):
# Get the signature of the coroutine method to start # Get the signature of the coroutine method to start
@ -230,6 +232,6 @@ class TaskManager:
for callback in self.task_status_change_callbacks: for callback in self.task_status_change_callbacks:
callback(name, kwargs_updated) callback(name, kwargs_updated)
else: else:
logger.error(f"Task `{name}` is already running!") logger.error("Task '%s' is already running!", name)
return start_task return start_task

View File

@ -234,7 +234,7 @@ class Server:
async def serve(self) -> None: async def serve(self) -> None:
process_id = os.getpid() process_id = os.getpid()
logger.info(f"Started server process [{process_id}]") logger.info("Started server process [%s]", process_id)
await self.startup() await self.startup()
if self.should_exit: if self.should_exit:
@ -242,7 +242,7 @@ class Server:
await self.main_loop() await self.main_loop()
await self.shutdown() 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 async def startup(self) -> None: # noqa: C901
self._loop = asyncio.get_running_loop() self._loop = asyncio.get_running_loop()
@ -330,7 +330,7 @@ class Server:
}, },
) )
except Exception as e: 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()) self._loop.create_task(notify())
@ -349,7 +349,7 @@ class Server:
async def shutdown(self) -> None: async def shutdown(self) -> None:
logger.info("Shutting down") 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: if self._state_manager is not None:
self._state_manager.save_state() self._state_manager.save_state()
@ -366,9 +366,9 @@ class Server:
try: try:
await task await task
except asyncio.CancelledError: except asyncio.CancelledError:
logger.debug(f"Cancelled {server_name} server.") logger.debug("Cancelled '%s' server.", server_name)
except Exception as e: except Exception as e:
logger.warning(f"Unexpected exception: {e}.") logger.warning("Unexpected exception: %s", e)
async def __cancel_tasks(self) -> None: async def __cancel_tasks(self) -> None:
for task in asyncio.all_tasks(self._loop): for task in asyncio.all_tasks(self._loop):
@ -376,9 +376,9 @@ class Server:
try: try:
await task await task
except asyncio.CancelledError: except asyncio.CancelledError:
logger.debug(f"Cancelled task {task.get_coro()}.") logger.debug("Cancelled task '%s'.", task.get_coro())
except Exception as e: except Exception as e:
logger.warning(f"Unexpected exception: {e}.") logger.exception("Unexpected exception: %s", e)
def install_signal_handlers(self) -> None: def install_signal_handlers(self) -> None:
if threading.current_thread() is not threading.main_thread(): 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: def handle_exit(self, sig: int = 0, frame: Optional[FrameType] = None) -> None:
if self.should_exit and sig == signal.SIGINT: 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) os._exit(1)
else: else:
self.should_exit = True 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( def custom_exception_handler(
self, loop: asyncio.AbstractEventLoop, context: dict[str, Any] self, loop: asyncio.AbstractEventLoop, context: dict[str, Any]
@ -421,7 +423,7 @@ class Server:
}, },
) )
except Exception as e: 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()) loop.create_task(emit_exception())
else: else:

View File

@ -107,7 +107,7 @@ class WebAPI:
@sio.event # type: ignore @sio.event # type: ignore
def set_attribute(sid: str, data: UpdateDict) -> Any: 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 = [*data["parent_path"].split("."), data["name"]]
path_list.remove("DataService") # always at the start, does not do anything path_list.remove("DataService") # always at the start, does not do anything
path = ".".join(path_list) path = ".".join(path_list)
@ -117,7 +117,7 @@ class WebAPI:
@sio.event # type: ignore @sio.event # type: ignore
def run_method(sid: str, data: RunMethodDict) -> Any: 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 = [*data["parent_path"].split("."), data["name"]]
path_list.remove("DataService") # always at the start, does not do anything path_list.remove("DataService") # always at the start, does not do anything
method = get_object_attr_from_path_list(self.service, path_list) method = get_object_attr_from_path_list(self.service, path_list)

View File

@ -59,7 +59,7 @@ def get_object_attr_from_path_list(target_obj: Any, path: list[str]) -> Any:
target_obj = getattr(target_obj, part) target_obj = getattr(target_obj, part)
except AttributeError: except AttributeError:
# The attribute doesn't exist # 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 None
return target_obj return target_obj
@ -141,7 +141,7 @@ def update_value_if_changed(
if getattr(target, attr_name_or_index) != new_value: if getattr(target, attr_name_or_index) != new_value:
setattr(target, attr_name_or_index, new_value) setattr(target, attr_name_or_index, new_value)
else: 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]]: 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(): if index_part.isdigit():
index = int(index_part) index = int(index_part)
else: 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 return attr_name, index

View File

@ -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"] and type(__value).__name__ not in ["CallbackManager", "TaskManager", "Quantity"]
): ):
logger.warning( logger.warning(
f"Warning: Class {type(__value).__name__} does not inherit from DataService." "Warning: Class '%s' does not inherit from DataService.",
type(__value).__name__,
) )

View File

@ -162,7 +162,7 @@ def test_load_state(tmp_path: Path, caplog: LogCaptureFixture):
"Ignoring value from JSON file..." "Ignoring value from JSON file..."
) in caplog.text ) in caplog.text
assert ( 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 "Ignoring value from JSON file..." in caplog.text
) )
assert "Value of attribute 'subservice.name' has not changed..." in caplog.text assert "Value of attribute 'subservice.name' has not changed..." in caplog.text

View File

@ -15,7 +15,7 @@ def test_setattr_warnings(caplog: LogCaptureFixture) -> None: # noqa
ServiceClass() 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 def test_private_attribute_warning(caplog: LogCaptureFixture) -> None: # noqa