labthings / labthings/labthings-fastapi

How should settings relate to a settings file on disk?

Open
#159 4 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
9
Forks
4
PR merge metrics
No merged PRs in 30d

Description

Settings are properties of a Thing that get synced to disk. I think there's a difference in how @julianstirling and I think about settings, which leads to us wanting to implement them differently. This issue is hopefully a place where we can record our discussion of how this should work, so that we can then make the implementation match that intention.

Currently, settings fit into the lifecycle of a Thing roughly as set out below:

  • Thing.__init__ is called to create a Thing instance. At this point, the Thing should not communicate with its hardware, and is not connected to a server.
  • ThingServer.add_thing is called, at which point the Thing is attached to the server. This, in turn, calls
    • Thing.attach_to_server which receives the settings file path as its arguments.
      • Thing.load_settings loads the settings from disk and assigns them to the relevant attributes
      • Thing.path is populated with the relative path of the Thing in the HTTP API
      • The various HTTP endpoints for the Thing Description, websocket, and properties and actions are created and added to the FastAPI app.
  • The server is started, trigering ThingServer.lifecycle
    • Thing.__enter__ is called. This is where the Thing should set itself up and initialise its hardware components (if any are present).
    • Everything should now be fully set up, and the server will run the Thing code and respond to HTTP requests.
      • Any time a setting is written to (via HTTP or by assigning to the attribute of the .Thing, all settings are read and combined into a dictionary, which is written to settings.json.
    • Once the server gets the signal to shut down, we stop responding to HTTP requests.
    • Thing.__exit__ is called, to instruct the Thing to shut down.
  • Everything should now be finished, and the script terminates.

There are a few points worth highlighting in the current implementation:

  • It's a bad idea to use settings before they have been loaded, so __init__ should not use the settings. They will have default values at this point, and anything written to the settings will be replaced by the settings file when it's loaded.
  • When the settings are loaded, the hardware will not yet have been initialised. This means that any code triggered by assigning to a setting will not be able to access the hardware.
  • Saving the settings file is triggered only by assigning to an attribute of the Thing or by explicitly calling Thing.save_settings. If settings are changed by another method, this won't be captured.

Saving settings

The current implementation updates the settings file each time a setting is assigned to. This means that, in most cases, settings.json stays up to date with the values of the settings. However, there are several ways in which it can become out of sync:

  • Mutable datatypes: a dict or list or BaseModel may be modified without attribute assignment (e.g. MyThing.myprop["foo"] = "bar" will not trigger a save).
  • Functional settings: settings that use a getter may change their value each time they are read. Current use cases for functional settings include:
    • Hardware settings: settings that are synchronised to hardware, i.e. writing to the setting causes the hardware to be updated with the new setting. If the hardware setting is changed by an other method, we must manually read it back.
    • Validation: settings are variable-like, but may raise an exception and not change their value if the new value is invalid. This causes no sync issues: the settings file will stay in sync with the attribute values.
    • Settings that aren't the single source of truth: if a value lives elsewhere (e.g. as an attribute of a plug-in class), a getter and setter can make it work like setting, but there's no notification if the underlying data is changed by another method.

Currently, the work-around for the various scenarios where the file becomes out of date is to manually call Thing.save_settings. This can be forgotten, and in the event of a server crash or improper shut-down it may not happen at all, resulting in things unexpectedly resetting.

It's also possible that, if settings are frequently updated or if many settings are changed at once, there will be many writes to the settings file which isn't ideal.

Looking at the way settings are currently used in the OpenFlexure Microscope, I think it is hard to envisage a way that settings can be guaranteed to stay in sync with the settings file. I think there are a few approaches we could take, each with their own issues:

  • Periodically sync the settings: rather than rely on every action of a Thing ensuring settings are saved, we could have a background process responsible for checking if settings have changed and syncing them if they have. This would ensure settings are never too stale, but as with any background process it could increase the load on the server and (more problematically) if getter code has side-effects, this could happen at unpredictable times.
  • Sync the settings after each action, or after actions that declare they might modify a setting. This is perhaps part-way between polling settings for changes and relying on manual saves, and might be a nice compromise. Doesn't help if settings might change on the hardware directly, or as a result of a background process.
  • Load and save the settings when the server is started and stopped. This is very simple, but will frequently fail if the server is stopped without a clean shutdown, which is not an infrequent occurrence.
  • Use only variable-like settings. This makes it easier to sync them with disk, but limits what settings may do. We would probably have to use properties that wrap variable-like settings in order to implement some more complicated behaviour. Still doesn't fix the problem of mutable datatypes.

Loading settings

Currently, settings that use a getter method to return their value must be initialised by calling a setter. It might be appropriate to initialise the settings with a separate method, allowing for read-only settings that still get synchronised to the settings file. It might also be nice to give the Thing an obvious hook to transfer settings to the hardware during __enter__, or at least to write a tutorial that covers how to neatly use settings that are synchronised with a piece of hardware.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing the Thing and ThingServer lifecycle described in the issue, especially attach_to_server, load_settings, save_settings, enter, exit, and ThingServer.lifecycle. Compare the listed loading and synchronization approaches, then document the chosen behavior and the implementation scope needed to keep settings and settings.json consistent.

Written by the indexing model from the issue text.

Assessment

Tech stack
fastapi, python
Domain
api, backend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.