Skip to content

Commit 580bdab

Browse files
Split reaidng settings file from loading settings.
The nested exception logic was getting confusing I worried the cognative complexity and the length of the function was too high. There is no reason to be in disable_saving_settings mode when loading the file. As such all of the file loading can be done before this starts. Moving it to it's own function provides simplifies it significantly.
1 parent b945a8d commit 580bdab

File tree

1 file changed

+47
-26
lines changed

1 file changed

+47
-26
lines changed

src/labthings_fastapi/thing.py

Lines changed: 47 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,50 @@ def thing_description(request: Request) -> ThingDescription:
188188
async def websocket(ws: WebSocket) -> None:
189189
await websocket_endpoint(self, ws)
190190

191+
def _read_settings_file(self) -> Mapping[str, Any] | None:
192+
"""Read the settings file and return a mapping of saved settings or None.
193+
194+
This function handles reading the settings from the disk. It is designed
195+
to be called by `load_settings`. Any exceptions caused by file handling or
196+
file corruption are caught and logged as warnings.
197+
198+
:return: A Mapping of setting name to setting value, or None if no settings
199+
could be read from file.
200+
"""
201+
setting_storage_path = self._thing_server_interface.settings_file_path
202+
thing_name = type(self).__name__
203+
if not os.path.exists(setting_storage_path):
204+
# If the settings file doesn't exist, we have nothing to do - the settings
205+
# are already initialised to their default values.
206+
return None
207+
208+
# Load the settings.
209+
try:
210+
with open(setting_storage_path, "r", encoding="utf-8") as file_obj:
211+
settings = json.load(file_obj)
212+
except (FileNotFoundError, PermissionError, JSONDecodeError):
213+
# Note that if the settings file is missing, we should already have
214+
# returned before attempting to load settings.
215+
self.logger.warning(
216+
"Error loading settings for %s from %s, could not load a JSON "
217+
"object. Settings for this Thing will be reset to default.",
218+
thing_name,
219+
setting_storage_path,
220+
)
221+
return None
222+
223+
if not isinstance(settings, Mapping):
224+
self.logger.warning(
225+
"Error loading settings for %s from %s. The file does not contain a "
226+
"Mapping",
227+
thing_name,
228+
setting_storage_path,
229+
)
230+
return None
231+
232+
# The settings are loaded and are a Mapping. Return them.
233+
return settings
234+
191235
def load_settings(self) -> None:
192236
"""Load settings from json.
193237
@@ -200,38 +244,15 @@ def load_settings(self) -> None:
200244
Note that no notifications will be triggered when the settings are set,
201245
so if action is needed (e.g. updating hardware with the loaded settings)
202246
it should be taken in ``__enter__``.
203-
204-
:raises TypeError: if the JSON file does not contain a dictionary. This is
205-
handled internally and logged, so the exception doesn't propagate
206-
outside of the function.
207247
"""
208-
setting_storage_path = self._thing_server_interface.settings_file_path
209-
thing_name = type(self).__name__
210-
if not os.path.exists(setting_storage_path):
211-
# If the settings file doesn't exist, we have nothing to do - the settings
212-
# are already initialised to their default values.
248+
settings = self._read_settings_file()
249+
if settings is None:
250+
# Return if no settings can be loaded from file.
213251
return
214252

215253
# Stop recursion by not allowing settings to be saved as we're reading them.
216254
self._disable_saving_settings = True
217255
try:
218-
# The inner try: block ensures we only catch these errors while loading the
219-
# file, not when applying the settings.
220-
try:
221-
with open(setting_storage_path, "r", encoding="utf-8") as file_obj:
222-
settings = json.load(file_obj)
223-
if not isinstance(settings, Mapping):
224-
raise TypeError("The settings file must be a JSON object.")
225-
except (FileNotFoundError, PermissionError, TypeError, JSONDecodeError):
226-
# Note that if the settings file is missing, we should already have
227-
# returned before attempting to load settings.
228-
self.logger.warning(
229-
"Error loading settings for %s from %s, could not load a JSON "
230-
"object. Settings for this Thing will be reset to default.",
231-
thing_name,
232-
setting_storage_path,
233-
)
234-
return
235256
for name, value in settings.items():
236257
try:
237258
setting = self.settings[name]

0 commit comments

Comments
 (0)