Skip to content

Commit 044d363

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 044d363

File tree

1 file changed

+44
-26
lines changed

1 file changed

+44
-26
lines changed

src/labthings_fastapi/thing.py

Lines changed: 44 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,47 @@ 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+
setting_storage_path = self._thing_server_interface.settings_file_path
199+
thing_name = type(self).__name__
200+
if not os.path.exists(setting_storage_path):
201+
# If the settings file doesn't exist, we have nothing to do - the settings
202+
# are already initialised to their default values.
203+
return None
204+
205+
# Load the settings.
206+
try:
207+
with open(setting_storage_path, "r", encoding="utf-8") as file_obj:
208+
settings = json.load(file_obj)
209+
except (FileNotFoundError, PermissionError, JSONDecodeError):
210+
# Note that if the settings file is missing, we should already have
211+
# returned before attempting to load settings.
212+
self.logger.warning(
213+
"Error loading settings for %s from %s, could not load a JSON "
214+
"object. Settings for this Thing will be reset to default.",
215+
thing_name,
216+
setting_storage_path,
217+
)
218+
return None
219+
220+
if not isinstance(settings, Mapping):
221+
self.logger.warning(
222+
"Error loading settings for %s from %s. The file does not contain a "
223+
"Mapping",
224+
thing_name,
225+
setting_storage_path,
226+
)
227+
return None
228+
229+
# The settings are loaded and are a Mapping. Return them.
230+
return settings
231+
191232
def load_settings(self) -> None:
192233
"""Load settings from json.
193234
@@ -200,38 +241,15 @@ def load_settings(self) -> None:
200241
Note that no notifications will be triggered when the settings are set,
201242
so if action is needed (e.g. updating hardware with the loaded settings)
202243
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.
207244
"""
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.
245+
settings = self._read_settings_file()
246+
if settings is None:
247+
# Return if no settings can be loaded from file.
213248
return
214249

215250
# Stop recursion by not allowing settings to be saved as we're reading them.
216251
self._disable_saving_settings = True
217252
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
235253
for name, value in settings.items():
236254
try:
237255
setting = self.settings[name]

0 commit comments

Comments
 (0)