Add application configuration that propagates to Things

[julianstirling]

Add an arbitrary configuration dictionary for downstream applications to use.

Context

In the OpenFlexure Microscope we have configuration specific to our application. We need to intercept the configuration validation and extract our own information. However, there is still no good way to propagate this information to individual Things.

This PR creates a place for an application_configuration, where downstream applications can put any configuration they find useful, and make it available to each Thing in the application.

No validation is performed, validation is left to downstream applications.

[barecheck]

Barecheck - Code coverage report

Total: 96.5%

Your code coverage diff: -0.02% ▾

Uncovered files and lines

File	Lines
src/labthings_fastapi/server/__init__.py	197, 358
src/labthings_fastapi/testing.py	130, 222

[rwb27]

I've skimmed the code and it looks very much like it does what it's supposed to with a minimum of fuss. Having a way to add config that's shared between Things is definitely a feature worth having, though it would be nice if it could be a bit more structured.

My initial thought was to suggest we use a Thing to manage that: the OpenFlexureApplicationThing could then expose the various things needed by everything else, and could be accessed through a thing slot. That keeps everything nicely typed. Unfortunately, at the moment there's no nice way to get information from the configuration file into a Thing, except init_kwargs. So, I suppose that could work, but it wouldn't be beautiful. I also don't think I've got a lot of testing going on for passing arguments to things from the config.

To make that slightly more concrete, how about:

class OFMApplicationThing(lt.Thing):
    def __init__(self, thing_server_interface, **settings):
        super().__init__(thing_server_interface)
        self._settings = **settings

    @property
    def data_folder(self) -> str:
        return self._settings.get("data_folder", "./data")

You could then reference this in the server config as something like

{
    "things": {
        "ofm_app": {
            "class": "path.to:OFMApplicationThing",
            "kwargs": {
                "data_folder": "/var/openflexure/data"
            }
        }
    }
}

One advantage of having a Thing manage the application config is that it gives an obvious place for validation logic, and would even allow settings as well as configuration. I'd be curious to see what you think of a solution like that.

I don't think there's anything fundamentally wrong with having somewhere to pass arbitrary information to Things from the config file - but doing it with a Thing does feel like it makes it easier to manage. I think the implementation in this MR is fine, but I can see it becoming a pain to add in validation nicely.

One thing I have wondered for a while is whether we could come up with a nice way to specify settings in the configuration, i.e. you could add a Thing, but specify the value of some or all of its settings. I think that gets complicated quite quickly: it would be fine for a data setting, and problematic for functional ones. Perhaps what would be helpful here is adding in another descriptor that is a bit like a setting, except it's read-only and set in the config rather than the settings file. Obviously that should be possible at the moment using kwargs when we create the Thing, but it might make the code a bit cleaner.

[julianstirling]

How validation is done is up to the downstream application. We need to remember that if this is a library that people are going to use in laboratories it needs to also support quick set up, and prototyping, as well as a final locked down application.

If LabThings is too opinionated on everything must be validated then it creating a huge adoption barrier. It also means that any use case that has not been considered becomes very difficult.

I totally agree that downstream code should find a way to validate its information, but that is the responsibility and choice of downstream applications to do what is appropriate for them at that time.

It is also worth noting that information is only available as a thing slot then it isn't available during __init__ or available to other code during setup.

[rwb27]

I think in principle this is a good thing to add, but I'm going to argue against making it an attribute of Thing, because I think it's clearer to keep it in Thing._thing_server_interface.application_config.

I also think the deepcopy should move to either ThingServer or ThingServerInterface because at present it will (1) proactively waste time copying stuff on Thing.__init__ and (2) allow each Thing to independently modify its own copy of the config. If a Thing wants to do that, I think it's appropriate for the downstream Thing to access it during __init__ (which is possible) and explicitly store its own copy.

I don't think either of those changes would cause any downstream problems, so I hope they're fairly uncontroversial.

[rwb27]

I'm going to assume that @julianstirling you're happy with the tweaks (I guess you can't approve as you made the PR), so I'll merge this now.

I suspect the test coverage decrease is due to the extra code in testing. I should probably add a test_testing module but I think I'll leave that for another day.