Drop dependencies feedback #191
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This marks a few more methods of ThingServer as private, and accepts a dict of Things to be created during initialisation. In order to do this cleanly, I have now formalised a schema for config files, using a Pydantic model. Tests (and no doubt fixes) will come in the next commit.
The config modules now do all the required validation, including Thing names, ensuring we expand classes into full `ThingConfig` objects, and supplying default values for args, kwargs, etc. I've exposed ThingConfig and ThingServerConfig at module level as they are likely useful in a number of scenarios, particularly writing tests. It is possible to supply a dict whenever a ThingConfig is required - I'll include this in the tests, but I don't want to recommend it - it's better to use the model as it makes it harder to miss fields.
There was duplicated validation code in ThingServer, which I've removed, in favour of using the ThingServerConfig model. I've also split up the code adding Things, so now we: 1. Create Things (and supply a ThingServerInterface) 2. Make connections between Things. 3. Add Things to the API. Step (3) may move out of __init__at some point in the future.
If the server is started from the command line, we print out any validation errors, then exit: a stack trace is not helpful. This also fixes an issue where ValidationError would not serialise properly, which stopped the test code working (it used multiprocessing).
I've removed ThingServer.add_thing in favour of passing a dictionary to ThingServer.__init__ and the test code is updated to reflect that. As a rule, when thing instances were required, I would: 1. Create the server 2. Assign `my_thing = server.things["my_thing"]` 3. Assert `assert isinstance(my_thing, MyThing)` That was enough for `pyright` (and I guess mypy) to correctly infer the type in my test code. It's slightly more verbose, but means we can use the same code in test and production, rather than needing a different way to create/add things in test code.
The name `thing_connection` confused people. `thing_slot` suggests we are marking the attribute and its value will be supplied later, which is exactly right. This commit makes that swap. I've searched/replaced every instance of ThingConnection and thing_connection: we should check the docs build OK, this is perhaps most easily done in CI.
This has been removed as its functionality is provided by `pydantic.ImportString`. It is no longer used for the fallback server: we gain nothing from the dynamic import. In the future, if we want to make it configurable, we could use `ImportString` there too.
Now that it's imported statically, `mypy` spots typing errors with the fallback server. These are now annotated correctly.
I've reworked core concepts and removed DirectThingClient, replacing it with a new "structure" page that I think preserves most of the still-useful content.
This was referenced Oct 28, 2025
rwb27
force-pushed
the
drop-dependencies-feedback
branch
from
b2c9dca to
aa04aff
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements the high level feedback from @julianstirling and @bprobert97 on my series of PRs to address #182 .
It makes two major API changes:
Thinginstances are now created inThingServer.__init__and connected immediately afterwards: this minimises the number of distinct phases in the Thing lifecycle, and avoids confusion in test code where a server might be in a partially initialised state.thing_connectionis renamed tothing_slotand the docs are updated to use language along the lines of "mark this as something the server should supply later"In making the first of these changes, I ended up tidying up the relevant methods in the server, and creating a
pydanticmodel for the server configuration. This simplifies validation of configuration, and also handles loading Thing classes from object reference strings. I've removed my home-made function that did that.While there are a lot of files changed, the majority of these are just changes to how Things are created in test code, and the name change for
thing_slot.