diff --git a/docs/source/actions.rst b/docs/source/actions.rst index ffad7b54..68e1d1b0 100644 --- a/docs/source/actions.rst +++ b/docs/source/actions.rst @@ -5,7 +5,7 @@ Actions Actions are the way `.Thing` objects are instructed to do things. In Python terms, any method of a `.Thing` that we want to be able to call over HTTP -should be decorated as an Action, using :deco:`.thing_action`. +should be decorated as an Action, using `.thing_action`. This page gives an overview of how actions are implemented in LabThings-FastAPI. :ref:`wot_cc` includes a section on :ref:`wot_actions` that introduces the general concept. @@ -58,6 +58,51 @@ The first is ``self`` (the first positional argument), which is always the supply resources needed by the action. Most often, this is a way of accessing other `.Things` on the same server. +.. action_logging: +Logging from actions +-------------------- +Action code should use `.Thing.logger` to log messages. This will be configured +to handle messages on a per-invocation basis and make them available when the action +is queried over HTTP. + +This may be used to display status updates to the user when an action takes +a long time to run, or it may simply be a helpful debugging aid. + +See :mod:`.logs` for details of how this is implemented. + +.. action_cancellation: +Cancelling actions +------------------ +If an action could run for a long time, it is useful to be able to cancel it +cleanly. LabThings makes provision for this by allowing actions to be cancelled +using a ``DELETE`` HTTP request. In order to allow an action to be cancelled, +you must give LabThings opportunities to interrupt it. This is most often done +by replacing a `time.sleep()` statement with `.cancellable_sleep()` which +is equivalent, but will raise an exception if the action is cancelled. + +For more advanced options, see `.invocation_contexts` for detail. + +.. invocation_context: +Invocation contexts +------------------- +Cancelling actions and capturing their logs requires action code to use a +specific logger and check for cancel events. This is done using `contextvars` +such that the action code can use module-level symbols rather than needing +to explicitly pass the logger and cancel hook as arguments to the action +method. + +Usually, you don't need to consider this mechanism: simply use `.Thing.logger` +or `.cancellable_sleep` as explained above. However, if you want to run actions +outside of the server (for example, for testing purposes) or if you want to +call one action from another action, but not share the cancellation signal +or log, functions are provided in `.invocation_contexts` to manage this. + +If you start a new thread from an action, code running in that thread will +not have an invocation ID set in a context variable. A subclass of +`threading.Thread` is provided to do this, `.ThreadWithInvocationID`\ . +This may be useful for test code, or if you wish to run actions in the +background, with the option of cancelling them. + Raising exceptions ------------------ If an action raises an unhandled exception, the action will terminate with an Error diff --git a/docs/source/concurrency.rst b/docs/source/concurrency.rst index 6a5becf1..2a25b64d 100644 --- a/docs/source/concurrency.rst +++ b/docs/source/concurrency.rst @@ -22,3 +22,11 @@ Calling Things from other Things When one `Thing` calls the actions or properties of another `.Thing`, either directly or via a `.DirectThingClient`, no new threads are spawned: the action or property is run in the same thread as the caller. This mirrors the behaviour of the `.ThingClient`, which blocks until the action or property is complete. See :doc:`using_things` for more details on how to call actions and properties of other Things. +Invocations and concurrency +--------------------------- + +Each time an action is run ("invoked" in :ref:`wot_cc`), we create a new thread to run it. This thread has a context variable set, such that ``lt.cancellable_sleep`` and ``lt.get_invocation_logger`` are aware of which invocation is currently running. If an action spawns a new thread (e.g. using `threading.Thread`\ ), this new thread will not have an invocation ID, and consequently the two invocation-specific functions mentioned will not work. + +Usually, the best solution to this problem is to generate a new invocation ID for the thread. This means only the original action thread will receive cancellation events, and only the original action thread will log to the invocation logger. If the action is cancelled, you must cancel the background thread. This is the behaviour of `.ThreadWithInvocationID`\ . + +It is also possible to copy the current invocation ID to a new thread. This is often a bad idea, as it's ill-defined whether the exception will arise in the original thread or the new one if the invocation is cancelled. Logs from the two threads will also be interleaved. If it's desirable to log from the background thread, the invocation logger may safely be passed as an argument, rather than accessed via ``lt.get_invocation_logger``\ . diff --git a/docs/source/dependencies/dependencies.rst b/docs/source/dependencies/dependencies.rst index 88426e97..4e4823b2 100644 --- a/docs/source/dependencies/dependencies.rst +++ b/docs/source/dependencies/dependencies.rst @@ -3,11 +3,19 @@ Dependencies ============ +.. warning:: + + The use of dependencies is now deprecated. See :ref:`thing_slots` and `.ThingServerInterface` for a more intuitive way to access that functionality. + LabThings makes use of the powerful "dependency injection" mechanism in FastAPI. You can see the `FastAPI documentation`_ for more information. In brief, FastAPI dependencies are annotated types that instruct FastAPI to supply certain function arguments automatically. This removes the need to set up resources at the start of a function, and ensures everything the function needs is declared and typed clearly. The most common use for dependencies in LabThings is where an action needs to make use of another `.Thing` on the same `.ThingServer`. Inter-Thing dependencies ------------------------ +.. warning:: + + These dependencies are deprecated - see :ref:`thing_slots` instead. + Simple actions depend only on their input parameters and the `.Thing` on which they are defined. However, it's quite common to need something else, for example accessing another `.Thing` instance on the same LabThings server. There are two important principles to bear in mind here: * Other `.Thing` instances should be accessed using a `.DirectThingClient` subclass if possible. This creates a wrapper object that should work like a `.ThingClient`, meaning your code should work either on the server or in a client script. This makes the code much easier to debug. diff --git a/docs/source/dependencies/example.py b/docs/source/dependencies/example.py index 315611e2..e2c383a3 100644 --- a/docs/source/dependencies/example.py +++ b/docs/source/dependencies/example.py @@ -18,9 +18,12 @@ def increment_counter(self, my_thing: MyThingDep) -> None: my_thing.increment_counter() -server = lt.ThingServer() -server.add_thing("mything", MyThing) -server.add_thing("testthing", TestThing) +server = lt.ThingServer( + { + "mything": MyThing, + "testthing": TestThing, + } +) if __name__ == "__main__": import uvicorn diff --git a/docs/source/index.rst b/docs/source/index.rst index 81c9301d..f43b163f 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -7,10 +7,11 @@ Documentation for LabThings-FastAPI quickstart/quickstart.rst wot_core_concepts.rst - lt_core_concepts.rst + structure.rst tutorial/index.rst examples.rst actions.rst + thing_slots.rst dependencies/dependencies.rst blobs.rst concurrency.rst @@ -19,26 +20,21 @@ Documentation for LabThings-FastAPI autoapi/index -`labthings-fastapi` implements a Web of Things interface for laboratory hardware using Python. This is a ground-up rewrite of python-labthings_, replacing Flask 1 and Marshmallow with FastAPI and Pydantic. It is the underlying framework for v3 of the `OpenFlexure Microscope software `_. +`labthings-fastapi` is a Python library to simplify the process of making laboratory instruments available via a HTTP. It aims to create an API that is usable from any modern programming language, with API documentation in both :ref:`openapi` and :ref:`gen_td` formats. It is the underlying framework for v3 of the `OpenFlexure Microscope software `_. Key features and design aims are: -`labthings-fastapi` aims to simplify the process of making laboratory instruments available via an HTTP API. Key features and design aims are below: - -* Functionality together in `Thing` subclasses, which represent units of hardware or software (see :doc:`wot_core_concepts`) -* Methods and properties of `Thing` subclasses may be added to the HTTP API and Thing Description using decorators +* The functionality of a unit of hardware or software is described using `.Thing` subclasses. +* Methods and properties of `.Thing` subclasses may be added to the HTTP API and associated documentation using decorators. +* Datatypes of action input/outputs and properties are defined with Python type hints. +* Actions are decorated methods of a `.Thing` class. There is no need for separate schemas or endpoint definitions. +* Properties are defined either as typed attributes (similar to `pydantic` or `dataclasses`) or with a `property`\ -like decorator. +* Lifecycle and concurrency are appropriate for hardware: `Thing` code is always run in a thread, and each `Thing` is instantiated, started up, and shut down only once. * Vocabulary and concepts are aligned with the `W3C Web of Things `_ standard (see :doc:`wot_core_concepts`) - - Things are classes, with properties and actions defined exactly once - - Thing Descriptions are automatically generated, and validated with `pydantic` - - OpenAPI documentation is automatically generated by FastAPI -* We follow FastAPI_'s lead and try to use standard Python features to minimise unnecessary code - - Datatypes of action input/outputs and properties are defined with Python type hints - - Actions are defined exactly once, as a method of a `Thing` class - - Properties and actions are declared using decorators (or descriptors if that's preferred) - - FastAPI_ "Dependency injection" is used to manage relationships between Things and dependency on the server -* Lifecycle and concurrency are appropriate for hardware: `Thing` code is always run in a thread, and each `Thing` is instantiated and shut down only once. - - Starlette (used by FastAPI) can handle requests asynchronously - this improves performance and enables websockets and other long-lived connections. - - `Thing` code is still, for now, threaded. In the future it may become possible to us other concurrency models in `Thing` code. - -Compared to `python-labthings`_, this framework updates dependencies, shrinks the codebase, and simplifies the API (see :doc:`lt_core_concepts`). + +Previous version +---------------- + +This is a ground-up rewrite of python-labthings_, replacing Flask 1 and Marshmallow with FastAPI and Pydantic. +Compared to `python-labthings`_, this framework updates dependencies, shrinks the codebase, and simplifies the API (see :doc:`lt_structure`). * FastAPI more or less completely eliminates OpenAPI generation code from our codebase * Marshmallow schemas and endpoint classes are replaced with Python type hints, eliminating double- or triple-definition of actions and their inputs/outputs. * Thing Description generation is very much simplified by the new structure (multiple Things instead of one massive Thing with many extensions) diff --git a/docs/source/lt_core_concepts.rst b/docs/source/lt_core_concepts.rst deleted file mode 100644 index 08ef280d..00000000 --- a/docs/source/lt_core_concepts.rst +++ /dev/null @@ -1,61 +0,0 @@ -.. _labthings_cc: - -LabThings Core Concepts -======================= - -LabThings FastAPI is a ground-up rewrite of LabThings using FastAPI. Many of the core concepts from FastAPI such as dependency injection are used heavily - -The LabThings Server --------------------- - -At its core LabThings FastAPI is a server-based framework. To use LabThings FastAPI a LabThings Server is created, and `.Thing` objects are added to the the server to provide functionality. - -The server API is accessed over an HTTP requests, allowing client code (see below) to be written in any language that can send an HTTP request. - -Everything is a Thing ---------------------- - -As described in :ref:`wot_cc`, a Thing represents a piece of hardware or software. LabThings-FastAPI automatically generates a :ref:`wot_td` to describe each Thing. Each function offered by the Thing is either a Property or Action (LabThings-FastAPI does not yet support Events). These are termed "interaction affordances" in WoT_ terminology. - -Code on the LabThings FastAPI Server is composed of Things, however these can call generic Python functions/classes. The entire HTTP API served by the server is defined by `.Thing` objects. As such the full API is composed of the actions and properties (and perhaps eventually events) defined in each Thing. - -_`WoT`: wot_core_concepts - -Properties vs Settings ----------------------- - -A Thing in LabThings-FastAPI can have Settings as well as Properties. "Setting" is LabThings-FastAPI terminology for a "Property" with a value that persists after the server is restarted. All Settings are Properties, and -- except for persisting after a server restart -- Settings are identical to any other Properties. - -Client Code ------------ - -Clients or client code (Not to be confused with a `.ThingClient`, see below) is the terminology used to describe any software that uses HTTP requests to access the LabThing Server. Clients can be written in any language that supports an HTTP request. However, LabThings FastAPI provides additional functionality that makes writing client code in Python easier. - -ThingClients ------------- - -When writing client code in Python it would be possible to formulate every interaction as an HTTP request. This has two major downsides: - -1. The code must establish a new connection to the server for each request. -2. Each request is formulated as a string pointing to the endpoint and ``json`` headers for sending any data. This leads to very messy code. - -Ideally the client would be able to run the `Thing` object's actions and read its properties in native python code. However, as the client code is running in a different process, and probably in a different python environment (or even on a different machine entirely!) there is no way to directly import the Python objectfor the `Thing`. - -To mitigate this client code can ask the server for a description of all of a `Thing`'s properties and actions, this is known as a `ThingDescription`. From this `ThingDescription` the client code can dynamically generate a new object with methods matching each `ThingAction` and properties matching each `ThingProperty`. **This dynamically generated object is called a ThingClient**. - -The :class:`.ThingClient` also handle supplying certain arguments to ThingActions without them needing to be explicitly passed each time the method is called. More detail on this is provided in the :doc:`dependencies/dependencies` page. - -DirectThingClients ------------------- - -When writing code to run on the server one Thing will need to call another Thing. Ideally this code should be identical to code written in a client. This way the code can be prototyped in a client notebook before being ported to the server. - -It would be possible to directly call the Thing object, however in this case the Python API would not be the same as for client code, because the dependencies would not automatically be supplied. -**RICHARD, Are there other reasons too?** - -To provide the same interface in server code as is provided in client code LabThings FastAPI can dynamically create a new object with the same (or at least very similar) API as the `ThingClient`, this is called a **DirectThingClient**. - -The key difference between a `ThingClient` and a `DirectThingClient` is that the `ThingClient` calls the `Thing` over HTTP from client code, whereas the `DirectThingClient` calls directly through the Python API from within the Server. - - - diff --git a/docs/source/quickstart/counter.py b/docs/source/quickstart/counter.py index 8e4b566d..43fcffee 100644 --- a/docs/source/quickstart/counter.py +++ b/docs/source/quickstart/counter.py @@ -31,10 +31,7 @@ def slowly_increase_counter(self) -> None: if __name__ == "__main__": import uvicorn - server = lt.ThingServer() - - # The line below creates a TestThing instance and adds it to the server - server.add_thing("counter", TestThing) + server = lt.ThingServer({"counter": TestThing}) # We run the server using `uvicorn`: uvicorn.run(server.app, port=5000) diff --git a/docs/source/structure.rst b/docs/source/structure.rst new file mode 100644 index 00000000..ef813b1c --- /dev/null +++ b/docs/source/structure.rst @@ -0,0 +1,42 @@ +.. _labthings_cc: +.. _labthings_structure: + +LabThings structure +=================== + +LabThings is intended to simplify the process of making a piece of hardware available through an HTTP API and documenting that API with :ref:`gen_docs`\ . + +Server +------ + +LabThings is a server-based framework. +The `.ThingServer` creates and manages the `.Thing` instances that represent individual hardware or software units. The functionality of those `.Thing`\ s is accessed via HTTP requests, which can be made from a web browser, the command line, or any programming language with an HTTP library. + +LabThings-FastAPI is built on top of `fastapi`\ , which is a fast, modern HTTP framework. LabThings provides functionality to manage `.Thing`\ s and their actions, including: + +* Initialising, starting up, and shutting down the `.Thing` instances, so that hardware is correctly started up and shut down. +* Managing actions, including making logs and output values available over HTTP. +* Managing `.Blob` input and output (i.e. binary objects that are best not serialised to JSON). +* Generating a :ref:`gen_td` in addition to the :ref:`openapi` documentation produced by `fastapi`\ . +* Making connections between `.Thing` instances as required. + +`.Thing`\ s +----------- + +Each unit of hardware (or software) that should be exposed by the server is implemented as a subclass of `.Thing`\ . A `.Thing` subclass represents a particular type of instrument (whether hardware or software), and its functionality is described using actions and properties, described below. `.Thing`\ s don't have to correspond to separate pieces of hardware: it's possible (and indeed recommended) to use `.Thing` subclasses for software components, plug-ins, swappable modules, or anything else that needs to add functionality to the server. `.Thing`\ s may access each other's attributes, so you can write a `.Thing` that implements a particular measurement protocol or task, using hardware that's accessed through other `.Thing` instances on the server. Each `.Thing` is documented by a :ref:`gen_td` which outlines its features in a higher-level way than :ref:`openapi`\ . + +The attributes of a `.Thing` are made available over HTTP by decorating or marking them with the following functions: + +* `.property` may be used as a decorator analogous to Python's built-in ``@property``\ . It can also be used to mark class attributes as variables that should be available over HTTP. +* `.setting` works similarly to `.property` but it is persisted to disk when the server stops, so the value is remembered. +* `.thing_action` is a decorator that makes methods available over HTTP. +* `.thing_slot` tells LabThings to supply an instance of another `.Thing` at runtime, so your `.Thing` can make use of it. + +Client Code +----------- + +Client code can be written in any language that supports an HTTP request. However, LabThings FastAPI provides additional functionality that makes writing client code in Python easier. + +`.ThingClient` is a class that wraps up the required HTTP requests into a simpler interface. It can retrieve the :ref:`gen_td` over HTTP and use it to generate a new object with methods matching each `.thing_action` and properties matching each `.property`. + +While the current dynamic implementation of `.ThingClient` can be inspected with functions like `help` at runtime, it does not work well with static tools like `mypy` or `pyright`\ . In the future, LabThings should be able to generate static client code that works better with autocompletion and type checking. \ No newline at end of file diff --git a/docs/source/thing_slots.rst b/docs/source/thing_slots.rst new file mode 100644 index 00000000..244e9bdf --- /dev/null +++ b/docs/source/thing_slots.rst @@ -0,0 +1,88 @@ +.. thing_slots: + +Thing Slots +=========== + +It is often desirable for two Things in the same server to be able to communicate. +In order to do this in a nicely typed way that is easy to test and inspect, +LabThings-FastAPI provides `.thing_slot`\ . This allows a `.Thing` +to declare that it depends on another `.Thing` being present, and provides a way for +the server to automatically connect the two when the server is set up. + +Thing connections are set up **after** all the `.Thing` instances are initialised. +This means you should not rely on them during initialisation: if you attempt to +access a connection before it is available, it will raise an exception. The +advantage of making connections after initialisation is that we don't need to +worry about the order in which `.Thing`\ s are created. + +The following example shows the use of a `.thing_slot`: + +.. code-block:: python + + import labthings_fastapi as lt + + + class ThingA(lt.Thing): + "A class that doesn't do much." + + @lt.action + def say_hello(self) -> str: + "A canonical example function." + return "Hello world." + + + class ThingB(lt.Thing): + "A class that relies on ThingA." + + thing_a: ThingA = lt.thing_slot() + + @lt.action + def say_hello(self) -> str: + "I'm too lazy to say hello, ThingA does it for me." + return self.thing_a.say_hello() + + + things = {"thing_a": ThingA, "thing_b": ThingB} + server = lt.ThingServer(things) + + +In this example, ``ThingB.thing_a`` is the simplest form of `.thing_slot`: it +is type hinted as a `.Thing` subclass, and by default the server will look for the +instance of that class and supply it when the server starts. If there is no +matching `.Thing` or if more than one instance is present, the server will fail +to start with a `.ThingSlotError`\ . + +It is also possible to use an optional type hint (``ThingA | None``), which +means there will be no error if a matching `.Thing` instance is not found, and +the slot will evaluate to `None`\ . Finally, a `.thing_slot` may be +type hinted as ``Mapping[str, ThingA]`` which permits zero or more instances to +be connected. The mapping keys are the names of the things. + +Configuring Thing Slots +----------------------- + +A `.thing_slot` may be given a default value. If this is a string, the server +will look up the `.Thing` by name. If the default is `None` the slot will +evaluate to `None` unless explicitly configured. + +Slots may also be specified in the server's configuration: +`.ThingConfig` takes an argument that allows connections to be made +by name (or set to `None`). The same field is present in a config +file. Each entry in the ``things`` list may have a ``thing_slots`` property +that sets up the connections. To repeat the example above with a configuration +file: + +.. code-block:: JSON + + "things": { + "thing_a": "example:ThingA", + "thing_b": { + "class": "example:ThingB", + "thing_slots": { + "thing_a": "thing_a" + } + } + } + +More detail can be found in the description of `.thing_slot` or the +:mod:`.thing_slots` module documentation. diff --git a/docs/source/tutorial/writing_a_thing.rst b/docs/source/tutorial/writing_a_thing.rst index defe6516..f6aa16bf 100644 --- a/docs/source/tutorial/writing_a_thing.rst +++ b/docs/source/tutorial/writing_a_thing.rst @@ -30,8 +30,7 @@ Our first Thing will pretend to be a light: we can set its brightness and turn i self.is_on = not self.is_on - server = lt.ThingServer() - server.add_thing("light", Light) + server = lt.ThingServer({"light": Light}) if __name__ == "__main__": import uvicorn diff --git a/src/labthings_fastapi/__init__.py b/src/labthings_fastapi/__init__.py index a7bbabe4..c84f3693 100644 --- a/src/labthings_fastapi/__init__.py +++ b/src/labthings_fastapi/__init__.py @@ -20,6 +20,7 @@ """ from .thing import Thing +from .thing_slots import thing_slot from .thing_server_interface import ThingServerInterface from .properties import property, setting, DataProperty, DataSetting from .decorators import ( @@ -30,7 +31,13 @@ from . import outputs from .outputs import blob from .server import ThingServer, cli +from .server.config_model import ThingConfig, ThingServerConfig from .client import ThingClient +from .invocation_contexts import ( + cancellable_sleep, + raise_if_cancelled, + ThreadWithInvocationID, +) # The symbols in __all__ are part of our public API. # They are imported when using `import labthings_fastapi as lt`. @@ -46,11 +53,17 @@ "DataProperty", "DataSetting", "thing_action", + "thing_slot", "fastapi_endpoint", "deps", "outputs", "blob", "ThingServer", "cli", + "ThingConfig", + "ThingServerConfig", "ThingClient", + "cancellable_sleep", + "raise_if_cancelled", + "ThreadWithInvocationID", ] diff --git a/src/labthings_fastapi/actions/__init__.py b/src/labthings_fastapi/actions/__init__.py index 098a92fc..cdca5895 100644 --- a/src/labthings_fastapi/actions/__init__.py +++ b/src/labthings_fastapi/actions/__init__.py @@ -19,24 +19,29 @@ import logging from collections import deque from threading import Thread, Lock -from typing import MutableSequence, Optional, Any +from typing import Optional, Any import uuid from typing import TYPE_CHECKING import weakref from fastapi import FastAPI, HTTPException, Request from pydantic import BaseModel +from labthings_fastapi.logs import add_thing_log_destination + from ..utilities import model_to_dict from ..utilities.introspection import EmptyInput from ..thing_description._model import LinkElement from .invocation_model import InvocationModel, InvocationStatus, LogRecordModel -from ..dependencies.invocation import ( - CancelHook, +from ..exceptions import ( InvocationCancelledError, InvocationError, - invocation_logger, ) from ..outputs.blob import BlobIOContextDep, blobdata_to_url_ctx +from ..invocation_contexts import ( + CancelEvent, + get_cancel_event, + set_invocation_id, +) if TYPE_CHECKING: # We only need these imports for type hints, so this avoids circular imports. @@ -77,7 +82,6 @@ def __init__( input: Optional[BaseModel] = None, dependencies: Optional[dict[str, Any]] = None, log_len: int = 1000, - cancel_hook: Optional[CancelHook] = None, ) -> None: """Create a thread to run an action and track its outputs. @@ -96,8 +100,6 @@ def __init__( FastAPI by its dependency injection mechanism. :param log_len: sets the number of log entries that will be held in memory by the invocation's logger. - :param cancel_hook: is a `threading.Event` subclass that tells the - invocation it's time to stop. See `.CancelHook`. """ Thread.__init__(self, daemon=True) @@ -106,7 +108,6 @@ def __init__( self.thing_ref = weakref.ref(thing) self.input = input if input is not None else EmptyInput() self.dependencies = dependencies if dependencies is not None else {} - self.cancel_hook = cancel_hook # A UUID for the Invocation (not the same as the threading.Thread ident) self._ID = id # Task ID @@ -195,14 +196,18 @@ def thing(self) -> Thing: raise RuntimeError("The `Thing` on which an action was run is missing!") return thing + @property + def cancel_hook(self) -> CancelEvent: + """The cancel event associated with this Invocation.""" + return get_cancel_event(self.id) + def cancel(self) -> None: """Cancel the task by requesting the code to stop. This is an opt-in feature: the action must use a `.CancelHook` dependency and periodically check it. """ - if self.cancel_hook is not None: - self.cancel_hook.set() + self.cancel_hook.set() def response(self, request: Optional[Request] = None) -> InvocationModel: """Generate a representation of the invocation suitable for HTTP. @@ -270,95 +275,57 @@ def run(self) -> None: # self.action evaluates to an ActionDescriptor. This confuses mypy, # which thinks we are calling ActionDescriptor.__get__. action: ActionDescriptor = self.action # type: ignore[call-overload] - # Create a logger just for this invocation, keyed to the invocation id - # Logs that go to this logger will be copied into `self._log` - handler = DequeLogHandler(dest=self._log) - logger = invocation_logger(self.id) - logger.addHandler(handler) - try: - action.emit_changed_event(self.thing, self._status.value) - - thing = self.thing - kwargs = model_to_dict(self.input) - if thing is None: # pragma: no cover - # The Thing is stored as a weakref, but it will always exist - # while the server is running - this error should never - # occur. - raise RuntimeError("Cannot start an invocation without a Thing.") - - with self._status_lock: - self._status = InvocationStatus.RUNNING - self._start_time = datetime.datetime.now() - action.emit_changed_event(self.thing, self._status.value) - - # The next line actually runs the action. - ret = action.__get__(thing)(**kwargs, **self.dependencies) - - with self._status_lock: - self._return_value = ret - self._status = InvocationStatus.COMPLETED - action.emit_changed_event(self.thing, self._status.value) - except InvocationCancelledError: - logger.info(f"Invocation {self.id} was cancelled.") - with self._status_lock: - self._status = InvocationStatus.CANCELLED - action.emit_changed_event(self.thing, self._status.value) - except Exception as e: # skipcq: PYL-W0703 - # First log - if isinstance(e, InvocationError): - # Log without traceback - logger.error(e) - else: - logger.exception(e) - # Then set status - with self._status_lock: - self._status = InvocationStatus.ERROR - self._exception = e + logger = self.thing.logger + # The line below saves records matching our ID to ``self._log`` + add_thing_log_destination(self.id, self._log) + with set_invocation_id(self.id): + try: action.emit_changed_event(self.thing, self._status.value) - finally: - with self._status_lock: - self._end_time = datetime.datetime.now() - self.expiry_time = self._end_time + datetime.timedelta( - seconds=self.retention_time, - ) - logger.removeHandler(handler) # Stop saving logs - # If we don't remove the log handler, it's a circular ref/memory leak. - -class DequeLogHandler(logging.Handler): - """A log handler that stores entries in memory.""" - - def __init__( - self, - dest: MutableSequence, - level: int = logging.INFO, - ) -> None: - """Set up a log handler that appends messages to a deque. - - .. warning:: - This log handler does not currently rotate or truncate - the list - so if you use it on a thread that produces a - lot of log messages, you may run into memory problems. - - Using a `.deque` with a finite capacity helps to mitigate - this. - - :param dest: should specify a deque, to which we will append - each log entry as it comes in. This is assumed to be thread - safe. - :param level: sets the level of the logger. For most invocations, - a log level of `logging.INFO` is appropriate. - """ - logging.Handler.__init__(self) - self.setLevel(level) - self.dest = dest - - def emit(self, record: logging.LogRecord) -> None: - """Save a log record to the destination deque. - - :param record: the `logging.LogRecord` object to add. - """ - self.dest.append(record) + thing = self.thing + kwargs = model_to_dict(self.input) + if thing is None: # pragma: no cover + # The Thing is stored as a weakref, but it will always exist + # while the server is running - this error should never + # occur. + raise RuntimeError("Cannot start an invocation without a Thing.") + + with self._status_lock: + self._status = InvocationStatus.RUNNING + self._start_time = datetime.datetime.now() + action.emit_changed_event(self.thing, self._status.value) + + bound_method = action.__get__(thing) + # Actually run the action + ret = bound_method(**kwargs, **self.dependencies) + + with self._status_lock: + self._return_value = ret + self._status = InvocationStatus.COMPLETED + action.emit_changed_event(self.thing, self._status.value) + except InvocationCancelledError: + logger.info(f"Invocation {self.id} was cancelled.") + with self._status_lock: + self._status = InvocationStatus.CANCELLED + action.emit_changed_event(self.thing, self._status.value) + except Exception as e: # skipcq: PYL-W0703 + # First log + if isinstance(e, InvocationError): + # Log without traceback + logger.error(e) + else: + logger.exception(e) + # Then set status + with self._status_lock: + self._status = InvocationStatus.ERROR + self._exception = e + action.emit_changed_event(self.thing, self._status.value) + finally: + with self._status_lock: + self._end_time = datetime.datetime.now() + self.expiry_time = self._end_time + datetime.timedelta( + seconds=self.retention_time, + ) class ActionManager: @@ -390,7 +357,6 @@ def invoke_action( id: uuid.UUID, input: Any, dependencies: dict[str, Any], - cancel_hook: CancelHook, ) -> Invocation: """Invoke an action, returning the thread where it's running. @@ -409,8 +375,6 @@ def invoke_action( keyword arguments. :param dependencies: is a dictionary of keyword arguments, supplied by FastAPI by its dependency injection mechanism. - :param cancel_hook: is a `threading.Event` subclass that tells the - invocation it's time to stop. See `.CancelHook`. :return: an `.Invocation` object that has been started. """ @@ -420,7 +384,6 @@ def invoke_action( input=input, dependencies=dependencies, id=id, - cancel_hook=cancel_hook, ) self.append_invocation(thread) thread.start() diff --git a/src/labthings_fastapi/base_descriptor.py b/src/labthings_fastapi/base_descriptor.py index a9fdf7a2..d2f66b4f 100644 --- a/src/labthings_fastapi/base_descriptor.py +++ b/src/labthings_fastapi/base_descriptor.py @@ -7,15 +7,18 @@ from __future__ import annotations import ast +import builtins import inspect from itertools import pairwise import textwrap from typing import overload, Generic, Mapping, TypeVar, TYPE_CHECKING from types import MappingProxyType -from weakref import WeakKeyDictionary +import typing +from weakref import WeakKeyDictionary, ref, ReferenceType from typing_extensions import Self from .utilities.introspection import get_docstring, get_summary +from .exceptions import MissingTypeError, InconsistentTypeError if TYPE_CHECKING: from .thing import Thing @@ -169,6 +172,7 @@ class Example: def __init__(self) -> None: """Initialise a BaseDescriptor.""" + super().__init__() self._name: str | None = None self._title: str | None = None self._description: str | None = None @@ -353,6 +357,176 @@ def instance_get(self, obj: Thing) -> Value: ) +class FieldTypedBaseDescriptor(Generic[Value], BaseDescriptor[Value]): + """A BaseDescriptor that determines its type like a dataclass field.""" + + def __init__(self) -> None: + """Initialise the FieldTypedBaseDescriptor. + + Very little happens at initialisation time: most of the type determination + happens in ``__set_name__`` and ``value_type`` so that type hints can + be lazily evaluated. + """ + super().__init__() + self._type: type | None = None # the type of the descriptor's value. + # It may be set during __set_name__ if a type is available, or the + # first time `self.value_type` is accessed. + self._unevaluated_type_hint: str | None = None # Set in `__set_name__` + # Type hints are not un-stringized in `__set_name__` but we remember them + # for later evaluation in `value_type`. + self._owner: ReferenceType[type] | None = None # For forward-reference types + # When we evaluate the type hints in `value_type` we need a reference to + # the object on which they are defined, to provide the context for the + # evaluation. + + def __set_name__(self, owner: type[Thing], name: str) -> None: + r"""Take note of the name and type. + + This function is where we determine the type of the property. It may + be specified in two ways: either by subscripting the descriptor + or by annotating the attribute. This example is for ``DataProperty`` + as this class is not intended to be used directly. + + .. code-block:: python + + class MyThing(Thing): + subscripted_property = DataProperty[int](0) + annotated_property: int = DataProperty(0) + + The second form often works better with autocompletion, though it + is usually called via a function to avoid type checking errors. + + Neither form allows us to access the type during ``__init__``, which + is why we find the type here. If there is a problem, exceptions raised + will appear to come from the class definition, so it's important to + include the name of the attribute. + + See :ref:`descriptors` for links to the Python docs about when + this function is called. + + For subscripted types (i.e. the first form above), we use + `typing.get_args` to retrieve the value type. This will be evaluated + immediately, resolving any forward references. + + We use `typing.get_type_hints` to resolve type hints on the owning + class. This takes care of a lot of subtleties like un-stringifying + forward references. In order to support forward references, we only + check for the existence of a type hint during ``__set_name__`` and + will evaluate it fully during ``value_type``\ . + + :param owner: the `.Thing` subclass to which we are being attached. + :param name: the name to which we have been assigned. + + :raises InconsistentTypeError: if the type is specified twice and + the two types are not identical. + :raises MissingTypeError: if no type hints have been given. + """ + # Call BaseDescriptor so we remember the name + super().__set_name__(owner, name) + + # Check for type subscripts + if hasattr(self, "__orig_class__"): + # We have been instantiated with a subscript, e.g. BaseProperty[int]. + # + # __orig_class__ is set on generic classes when they are instantiated + # with a subscripted type. It is not available during __init__, which + # is why we check for it here. + self._type = typing.get_args(self.__orig_class__)[0] + if isinstance(self._type, typing.ForwardRef): + raise MissingTypeError( + f"{owner}.{name} is a subscripted descriptor, where the " + f"subscript is a forward reference ({self._type}). Forward " + "references are not supported as subscripts." + ) + + # Check for annotations on the parent class + field_annotation = inspect.get_annotations(owner).get(name, None) + if field_annotation is not None: + # We have been assigned to an annotated class attribute, e.g. + # myprop: int = BaseProperty(0) + if self._type is not None and self._type != field_annotation: + # As a rule, if _type is already set, we don't expect any + # annotation on the attribute, so this error should not + # be a frequent occurrence. + raise InconsistentTypeError( + f"Property {name} on {owner} has conflicting types.\n\n" + f"The field annotation of {field_annotation} conflicts " + f"with the inferred type of {self._type}." + ) + self._unevaluated_type_hint = field_annotation + self._owner = ref(owner) + + # Ensure a type is specified. + # If we've not set _type by now, we are not going to set it, and the + # descriptor will not work properly. It's best to raise an error now. + # Note that we need to specify the attribute name, as the exception + # will appear to come from the end of the class definition, and not + # from the descriptor definition. + if self._type is None and self._unevaluated_type_hint is None: + raise MissingTypeError( + f"No type hint was found for attribute {name} on {owner}." + ) + + @builtins.property + def value_type(self) -> type[Value]: + """The type of this descriptor's value. + + This is only available after ``__set_name__`` has been called, which happens + at the end of the class definition. If it is called too early, a + `.DescriptorNotAddedToClassError` will be raised. + + Accessing this property will attempt to resolve forward references, + i.e. type annotations that are strings. If there is an error resolving + the forward reference, a `.MissingTypeError` will be raised. + + :return: the type of the descriptor's value. + :raises MissingTypeError: if the type is None, not resolvable, or not specified. + """ + self.assert_set_name_called() + if self._type is None and self._unevaluated_type_hint is not None: + # We have a forward reference, so we need to resolve it. + if self._owner is None: + raise MissingTypeError( + f"Can't resolve forward reference for type of {self.name} because " + "the class on which it was defined wasn't saved. This is a " + "LabThings bug - please report it." + ) + owner = self._owner() + if owner is None: + raise MissingTypeError( + f"Can't resolve forward reference for type of {self.name} because " + "the class on which it was defined has been garbage collected." + ) + try: + # Resolving a forward reference has quirks, and rather than tie us + # to undocumented implementation details of `typing` we just use + # `typing.get_type_hints`. + # This isn't efficient (it resolves everything, rather than just + # the one annotation we need, and it traverses the MRO when we know + # the class we're defined on) but it is part of the public API, + # and therefore much less likely to break. + # + # Note that we already checked there was an annotation in + # __set_name__. + hints = typing.get_type_hints(owner, include_extras=True) + self._type = hints[self.name] + except Exception as e: + raise MissingTypeError( + f"Can't resolve forward reference for type of {self.name}." + ) from e + if self._type is None: + # We should never reach this line: if `__set_name__` was called, we'd + # have raised an exception there if _type was None. If `__set_name__` + # has not been called, `self.assert_set_name_called()` would have failed. + # This block is required for `mypy` to know that self._type is not None. + raise MissingTypeError( + f"No type hint was found for property {self.name}. This may indicate " + "a bug in LabThings, as the error should have been caught before now." + ) + + return self._type + + # get_class_attribute_docstrings is a relatively expensive function that # will be called potentially quite a few times on the same class. It will # return the same result each time (because it depends only on the source diff --git a/src/labthings_fastapi/dependencies/invocation.py b/src/labthings_fastapi/dependencies/invocation.py index 8ed80f7a..7cf57896 100644 --- a/src/labthings_fastapi/dependencies/invocation.py +++ b/src/labthings_fastapi/dependencies/invocation.py @@ -36,7 +36,8 @@ from typing import Annotated from fastapi import Depends import logging -import threading +from ..invocation_contexts import CancelEvent +from ..logs import THING_LOGGER def invocation_id() -> uuid.UUID: @@ -87,9 +88,7 @@ def invocation_logger(id: InvocationID) -> logging.Logger: :return: A `logging.Logger` object specific to this invocation. """ - logger = logging.getLogger(f"labthings_fastapi.actions.{id}") - logger.setLevel(logging.INFO) - return logger + return THING_LOGGER.getChild("OLD_DEPENDENCY_LOGGER") InvocationLogger = Annotated[logging.Logger, Depends(invocation_logger)] @@ -100,83 +99,6 @@ def invocation_logger(id: InvocationID) -> logging.Logger: """ -class InvocationCancelledError(BaseException): - """An invocation was cancelled by the user. - - Note that this inherits from BaseException so won't be caught by - `except Exception`, it must be handled specifically. - - Action code may want to handle cancellation gracefully. This - exception should be propagated if the action's status should be - reported as ``cancelled``, or it may be handled so that the - action finishes, returns a value, and is marked as ``completed``. - - If this exception is handled, the `.CancelEvent` should be reset - to allow another `.InvocationCancelledError` to be raised if the - invocation receives a second cancellation signal. - """ - - -class InvocationError(RuntimeError): - """The invocation ended in an anticipated error state. - - When this error is raised, action execution stops as expected. The exception will be - logged at error level without a traceback, and the invocation will return with - error status. - - Subclass this error for errors that do not need further traceback information - to be provided with the error message in logs. - """ - - -class CancelEvent(threading.Event): - """An Event subclass that enables cancellation of actions. - - This `threading.Event` subclass adds methods to raise - `.InvocationCancelledError` exceptions if the invocation is cancelled, - usually by a ``DELETE`` request to the invocation's URL. - """ - - def __init__(self, id: InvocationID) -> None: - """Initialise the cancellation event. - - :param id: The invocation ID, annotated as a dependency so it is - supplied automatically by FastAPI. - """ - threading.Event.__init__(self) - self.invocation_id = id - - def raise_if_set(self) -> None: - """Raise an exception if the event is set. - - This is intended as a compact alternative to: - - .. code-block:: - - if cancel_event.is_set(): - raise InvocationCancelledError() - - :raise InvocationCancelledError: if the event has been cancelled. - """ - if self.is_set(): - raise InvocationCancelledError("The action was cancelled.") - - def sleep(self, timeout: float) -> None: - r"""Sleep for a given time in seconds, but raise an exception if cancelled. - - This function can be used in place of `time.sleep`. It will usually behave - the same as `time.sleep`\ , but if the cancel event is set during the time - when we are sleeping, an exception is raised to interrupt the sleep and - cancel the action. - - :param timeout: The time to sleep for, in seconds. - - :raise InvocationCancelledError: if the event has been cancelled. - """ - if self.wait(timeout): - raise InvocationCancelledError("The action was cancelled.") - - def invocation_cancel_hook(id: InvocationID) -> CancelHook: """Make a cancel hook for a particular invocation. diff --git a/src/labthings_fastapi/descriptors/action.py b/src/labthings_fastapi/descriptors/action.py index 1c3fa332..cd2ef6db 100644 --- a/src/labthings_fastapi/descriptors/action.py +++ b/src/labthings_fastapi/descriptors/action.py @@ -267,7 +267,6 @@ def start_action( input=body, dependencies=dependencies, id=id, - cancel_hook=cancel_hook, ) background_tasks.add_task(action_manager.expire_invocations) return action.response(request=request) diff --git a/src/labthings_fastapi/exceptions.py b/src/labthings_fastapi/exceptions.py index 2f81bfe3..fc99dba9 100644 --- a/src/labthings_fastapi/exceptions.py +++ b/src/labthings_fastapi/exceptions.py @@ -1,13 +1,8 @@ """A submodule for custom LabThings-FastAPI Exceptions.""" -# The "import x as x" syntax means symbols are interpreted as being re-exported, -# so they won't be flagged as unused by the linter. + # An __all__ for this module is less than helpful, unless we have an # automated check that everything's included. -from .dependencies.invocation import ( - InvocationCancelledError as InvocationCancelledError, -) -from .dependencies.invocation import InvocationError as InvocationError class NotConnectedToServerError(RuntimeError): @@ -51,3 +46,94 @@ class PropertyNotObservableError(RuntimeError): observable: functional properties (using a getter/setter) may not be observed. """ + + +class InconsistentTypeError(TypeError): + """Different type hints have been given for a descriptor. + + Some descriptors in LabThings, particularly `.DataProperty` and `.ThingSlot` + may have their type specified in different ways. If multiple type hints are + provided, they must match. See `.property` for more details. + """ + + +class MissingTypeError(TypeError): + """No type hints have been given for a descriptor that requires a type. + + Every property and thing connection should have a type hint, + There are different ways of providing these type hints. + This error indicates that no type hint was found. + + See documentation for `.property` and `.thing_slot` for more details. + """ + + +class ThingNotConnectedError(RuntimeError): + r"""`.ThingSlot`\ s have not yet been set up. + + This error is raised if a `.ThingSlot` is accessed before the `.Thing` has + been supplied by the LabThings server. This usually happens because either + the `.Thing` is being used without a server (in which case the attribute + should be mocked), or because it has been accessed before ``__enter__`` + has been called. + """ + + +class ThingSlotError(RuntimeError): + """A `.ThingSlot` could not be set up. + + This error is raised if the LabThings server is unable to set up a + `.ThingSlot`, for example because the named Thing does not exist, + or is of the wrong type, or is not specified and there is no default. + """ + + +class InvocationCancelledError(BaseException): + """An invocation was cancelled by the user. + + Note that this inherits from BaseException so won't be caught by + `except Exception`, it must be handled specifically. + + Action code may want to handle cancellation gracefully. This + exception should be propagated if the action's status should be + reported as ``cancelled``, or it may be handled so that the + action finishes, returns a value, and is marked as ``completed``. + + If this exception is handled and not re-raised, or if it arises in + a manually-created thread, the action will continue as normal. It + is a good idea to make sure your action terminates soon after this + exception is raised. + """ + + +class InvocationError(RuntimeError): + """The invocation ended in an anticipated error state. + + When this error is raised, action execution stops as expected. The exception will be + logged at error level without a traceback, and the invocation will return with + error status. + + Subclass this error for errors that do not need further traceback information + to be provided with the error message in logs. + """ + + +class NoInvocationContextError(RuntimeError): + """An invocation-specific resource has been requested from outside an invocation. + + This error is raised when the current invocation ID is requested, and there is no + current invocation ID. Invocation ID is determined from context (using a + `.ContextVar` ) and is available from within action functions. + + To avoid this error in test code or manually created threads, you should supply + an invocation context. + """ + + +class LogConfigurationError(RuntimeError): + """There is a problem with logging configuration. + + LabThings uses the `logging` module to collect logs from actions. This requires + certain handlers and filters to be set up. This exception is raised if they + cannot be added, or if they are not present when they are needed. + """ diff --git a/src/labthings_fastapi/invocation_contexts.py b/src/labthings_fastapi/invocation_contexts.py new file mode 100644 index 00000000..b7c31ae5 --- /dev/null +++ b/src/labthings_fastapi/invocation_contexts.py @@ -0,0 +1,369 @@ +r"""Invocation-specific resources provided via context. + +This module provides key resources to code that runs as part of an action, +specifically a mechanism to allow cancellation, and a way to manage logging. +These replace the old dependencies ``CancelHook`` and ``InvocationLogger``\ . + +If you are writing action code and want to use logging or allow cancellation, +most of the time you should just use `.get_invocation_logger` or +`.cancellable_sleep` which are exposed as part of the top-level module. + +This module includes lower-level functions that are useful for testing or +managing concurrency. Many of these accept an ``id`` argument, which is +optional. If it is not supplied, we will use the context variables to find +the current invocation ID. +""" + +from collections.abc import Iterator, Mapping, Sequence +from contextvars import ContextVar +from contextlib import contextmanager +from threading import Event, Thread +import time +from typing import Any, Callable +from typing_extensions import Self +from uuid import UUID, uuid4 +from weakref import WeakValueDictionary + +from .exceptions import InvocationCancelledError, NoInvocationContextError + + +invocation_id_ctx = ContextVar[UUID]("invocation_id_ctx") +"""Context variable storing the current invocation ID. + +Note that it is best not to access this directly. Using `.set_invocation_id` +is safer, as it ensures proper clean-up and continuity of the cancel event +associated with the invocation. +""" + + +def get_invocation_id() -> UUID: + """Return the current InvocationID. + + This function returns the ID of the current invocation. This is determined + from execution context: it will only succeed if it is called from an action + thread. + + If this function is called outside of an action thread, it will raise + an error. + + :return: the invocation ID of the current invocation. + :raises NoInvocationContextError: if called outside of an action thread. + """ + try: + return invocation_id_ctx.get() + except LookupError as e: + msg = "There is no invocation ID to return: this code was called from " + msg += "outside of an action thread." + raise NoInvocationContextError(msg) from e + + +@contextmanager +def set_invocation_id(id: UUID) -> Iterator[None]: + """Set the invocation ID associated with the current context. + + This is the preferred way to create a new invocation context. As well + as setting and cleaning up the invocation ID context variable, this + context manager ensures that the cancellation event persists and is + not accidentally reset because it's gone out of scope. + + :param id: The invocation ID to save in the context variable. + """ + token = invocation_id_ctx.set(id) + event = get_cancel_event(id) + try: + yield + finally: + invocation_id_ctx.reset(token) + del event + + +@contextmanager +def fake_invocation_context() -> Iterator[UUID]: + """Set a dummy invocation ID for a block of code. + + This function should be used in a ``with:`` block. + + :yields: the created invocation ID. + """ + id = uuid4() + with set_invocation_id(id): + yield id + + +class CancelEvent(Event): + """An Event subclass that enables cancellation of actions. + + This `threading.Event` subclass adds methods to raise + `.InvocationCancelledError` exceptions if the invocation is cancelled, + usually by a ``DELETE`` request to the invocation's URL. + """ + + _cancel_events: WeakValueDictionary[UUID, Self] = WeakValueDictionary() + "This class-level dictionary ensures only one event exists per invocation ID" + + def __init__(self, id: UUID) -> None: + """Initialise a cancellation event. + + Only one CancelEvent should exist per invocation. Trying to create a + second will raise an error. To avoid this, please use + `.CancelEvent.get_for_id` instead of the constructor. + + :param id: The invocation ID. + :raises RuntimeError: if a `.CancelEvent` has already been created for + the specified invocation ID. + """ + super().__init__() + self.invocation_id = id + if id in self.__class__._cancel_events: + msg = f"Tried to create a second CancelEvent for invocation {id}. " + msg += "Use `CancelEvent.get_for_id` to avoid this error." + raise RuntimeError(msg) + self.__class__._cancel_events[id] = self + + @classmethod + def get_for_id(cls, id: UUID) -> Self: + """Obtain the `.CancelEvent` for a particular Invocation ID. + + This is a safe way to obtain an instance of this class, though + the top-level function `.get_cancel_event` is recommended. + + Only one `.CancelEvent` should exist per Invocation. This method + will either create one, or return the existing one. + + :param id: The invocation ID. + :return: the cancel event for the given ``id`` . + """ + try: + return cls._cancel_events[id] + except KeyError: + return cls(id) + + def raise_if_set(self) -> None: + """Raise an exception if the event is set. + + An exception will be raised if the event has been set. + Before raising the exception, we clear the event. This means that setting + the event should raise exactly one exception, and that handling the exception + should result in the action continuing to run. + + This is intended as a compact alternative to: + + .. code-block:: + + if cancel_event.is_set(): + cancel_event.clear() + raise InvocationCancelledError() + + :raise InvocationCancelledError: if the event has been cancelled. + """ + if self.is_set(): + self.clear() + raise InvocationCancelledError("The action was cancelled.") + + def sleep(self, timeout: float) -> None: + r"""Sleep for a given time in seconds, but raise an exception if cancelled. + + This function can be used in place of `time.sleep`. It will usually behave + the same as `time.sleep`\ , but if the cancel event is set during the time + when we are sleeping, an exception is raised to interrupt the sleep and + cancel the action. The event is cleared before raising the exception. This + means that handling the exception is sufficient to allow the action to + continue. + + :param timeout: The time to sleep for, in seconds. + + :raise InvocationCancelledError: if the event has been cancelled. + """ + if self.wait(timeout): + self.clear() + raise InvocationCancelledError("The action was cancelled.") + + +def get_cancel_event(id: UUID | None = None) -> CancelEvent: + """Obtain an event that permits actions to be cancelled. + + :param id: The invocation ID. This will be determined from + context if not supplied. + :return: an event that allows the current invocation to be cancelled. + """ + if id is None: + id = get_invocation_id() + return CancelEvent.get_for_id(id) + + +def cancellable_sleep(interval: float) -> None: + r"""Sleep for a specified time, allowing cancellation. + + This function should be called from action functions instead of + `time.sleep` to allow them to be cancelled. Usually, this + function is equivalent to `time.sleep` (it waits the specified + number of seconds). If the action is cancelled during the sleep, + it will raise an `.InvocationCancelledError` to signal that the + action should finish. + + .. warning:: + + This function uses `.Event.wait` internally, which suffers + from timing errors on some platforms: it may have error of + around 10-20ms. If that's a problem, consider using + `time.sleep` instead. ``lt.raise_if_cancelled()`` may then + be used to allow cancellation. + + If this function is called from outside of an action thread, it + will revert to `time.sleep`\ . + + :param interval: The length of time to wait for, in seconds. + """ + try: + event = get_cancel_event() + event.sleep(interval) + except NoInvocationContextError: + time.sleep(interval) + + +def raise_if_cancelled() -> None: + """Raise an exception if the current invocation has been cancelled. + + This function checks for cancellation events and, if the current + action invocation has been cancelled, it will raise an + `.InvocationCancelledError` to signal the thread to terminate. + It is equivalent to `.cancellable_sleep` but without waiting any + time. + + If called outside of an invocation context, this function does + nothing, and will not raise an error. + """ + try: + event = get_cancel_event() + event.raise_if_set() + except NoInvocationContextError: + pass + + +class ThreadWithInvocationID(Thread): + r"""A thread that sets a new invocation ID. + + This is a subclass of `threading.Thread` and works very much the + same way. It implements its functionality by overriding the ``run`` + method, so this should not be overridden again - you should instead + specify the code to run using the ``target`` argument. + + This function enables an action to be run in a thread, which gets its + own invocation ID and cancel hook. This means logs will not be interleaved + with the calling action, and the thread may be cancelled just like an + action started over HTTP, by calling its ``cancel`` method. + + The thread also remembers the return value of the target function + in the property ``result`` and stores any exception raised in the + ``exception`` property. + + A final LabThings-specific feature is cancellation propagation. If + the thread is started from an action that may be cancelled, it may + be joined with ``join_and_propagate_cancel``\ . This is intended + to be equivalent to calling ``join`` but with the added feature that, + if the parent thread is cancelled while waiting for the child thread + to join, the child thread will also be cancelled. + """ + + def __init__( + self, + target: Callable, + args: Sequence[Any] | None = None, + kwargs: Mapping[str, Any] | None = None, + *super_args: Any, + **super_kwargs: Any, + ) -> None: + r"""Initialise a thread with invocation ID. + + :param target: the function to call in the thread. + :param args: positional arguments to ``target``\ . + :param kwargs: keyword arguments to ``target``\ . + :param \*super_args: arguments passed to `threading.Thread`\ . + :param \*\*super_kwargs: keyword arguments passed to `threading.Thread`\ . + """ + super().__init__(*super_args, **super_kwargs) + self._target = target + self._args = args or [] + self._kwargs = kwargs or {} + self._invocation_id: UUID = uuid4() + self._result: Any = None + self._exception: BaseException | None = None + # We hold a reference to the CancelEvent below, to ensure that it + # doesn't get garbage collected. Garbage collection means that we + # might (or might not) ignore cancellation that happens before the + # thread has started properly. This is an edge case, but can mess + # up testing code, so it's safest to ensure the event exists for at + # least as long as this Thread. + self._cancel_event = CancelEvent.get_for_id(self._invocation_id) + + @property + def invocation_id(self) -> UUID: + """The InvocationID of this thread.""" + return self._invocation_id + + @property + def result(self) -> Any: + """The return value of the target function.""" + return self._result + + @property + def exception(self) -> BaseException | None: + """The exception raised by the target function, or None.""" + return self._exception + + def cancel(self) -> None: + """Set the cancel event to tell the code to terminate.""" + get_cancel_event(self.invocation_id).set() + + def join_and_propagate_cancel(self, poll_interval: float = 0.2) -> None: + """Wait for the thread to finish, and propagate cancellation. + + This function wraps `threading.Thread.join` but periodically checks if + the calling thread has been cancelled. If it has, it will cancel the + thread, before attempting to ``join`` it again. + + Note that, if the invocation that calls this function is cancelled + while the function is running, the exception will propagate, i.e. + you should handle `.InvocationCancelledError` unless you wish + your invocation to terminate if it is cancelled. + + :param poll_interval: How often to check for cancellation of the + calling thread, in seconds. + :raises InvocationCancelledError: if this invocation is cancelled + while waiting for the thread to join. + """ + cancellation: InvocationCancelledError | None = None + self._polls = 0 + self._attempted_cancels = 0 + while self.is_alive(): + try: + raise_if_cancelled() + self._polls += 1 + except InvocationCancelledError as e: + # Propagate the cancellation signal to the thread + cancellation = e + self.cancel() + self._attempted_cancels += 1 + self.join(timeout=poll_interval) + if cancellation is not None: + # If the action was cancelled, propagate the cancellation + # after the thread has been joined. + # Note that, regardless of how many times the thread was + # cancelled, we will only raise one exception after the + # calling thread was joined. + raise InvocationCancelledError() from cancellation + + def run(self) -> None: + """Run the target function, with invocation ID set in the context variable.""" + try: + with set_invocation_id(self.invocation_id): + if self._target is not None: + self._result = self._target(*self._args, **self._kwargs) + except BaseException as e: # noqa: BLE001 + # This catch-all Except allows us to access exceptions + # in the parent thread + self._exception = e + finally: + # Avoid a refcycle if the thread is running a function with + # an argument that has a member that points to the thread. + del self._target, self._args, self._kwargs diff --git a/src/labthings_fastapi/logs.py b/src/labthings_fastapi/logs.py new file mode 100644 index 00000000..2fbeac60 --- /dev/null +++ b/src/labthings_fastapi/logs.py @@ -0,0 +1,131 @@ +"""Log-related functions and classes. + +This module currently contains code that allows us to filter out logs by invocaton +ID, so that they may be returned when invocations are queried. +""" + +from collections.abc import MutableSequence +import logging +from uuid import UUID +from weakref import WeakValueDictionary +from .invocation_contexts import get_invocation_id +from .exceptions import LogConfigurationError, NoInvocationContextError + + +THING_LOGGER = logging.getLogger("labthings_fastapi.things") + + +def inject_invocation_id(record: logging.LogRecord) -> bool: + r"""Add the invocation ID to records. + + This function adds the current invocation ID to log records. If it is not + available, we set the record's ``invocation_id`` property to `None`\ . + + :param record: the `logging.LogRecord` object to modify. + + :return: `True` (which signals we should keep every record if this is used + as a filter). + """ + try: + id = get_invocation_id() + record.invocation_id = id + except NoInvocationContextError: + record.invocation_id = None + return True + + +class DequeByInvocationIDHandler(logging.Handler): + """A log handler that stores entries in memory.""" + + def __init__( + self, + level: int = logging.INFO, + ) -> None: + """Set up a log handler that appends messages to a deque. + + .. warning:: + This log handler does not currently rotate or truncate + the list. It's best to use a `deque` with a finite capacity + to avoid memory leaks. + + :param level: sets the level of the logger. For most invocations, + a log level of `logging.INFO` is appropriate. + """ + super().__init__() + self.setLevel(level) + self.destinations = WeakValueDictionary[UUID, MutableSequence]() + self.addFilter(inject_invocation_id) + + def add_destination_for_id(self, id: UUID, destination: MutableSequence) -> None: + """Append logs matching ``id`` to a specified sequence. + + :param id: the ``invocation_id`` to match. + :param destination: should specify a deque, to which we will append + each log entry as it comes in. This is assumed to be thread + safe. + """ + self.destinations[id] = destination + + def emit(self, record: logging.LogRecord) -> None: + """Save a log record to the destination deque. + + :param record: the `logging.LogRecord` object to add. + """ + id = getattr(record, "invocation_id", None) + if isinstance(id, UUID): + try: + self.destinations[id].append(record) + except KeyError: + pass # If there's no destination for a particular log, ignore it. + + +def configure_thing_logger() -> None: + """Set up the logger for thing instances. + + We always set the logger for thing instances to level INFO, as this + is currently used to relay progress to the client. + + This function will collect logs on a per-invocation + basis by adding a `.DequeByInvocationIDHandler` to the log. Only one + such handler will be added - subsequent calls are ignored. + + Unfortunately, filters must be added to every sub-logger, so globally adding + a filter to add invocation ID is not possible. Instead, we attach a filter to + the handler, which filters all the records that propagate to it (i.e. anything + that starts with ``labthings_fastapi.things``). + """ + THING_LOGGER.setLevel(logging.INFO) + if not any( + isinstance(h, DequeByInvocationIDHandler) for h in THING_LOGGER.handlers + ): + THING_LOGGER.addHandler(DequeByInvocationIDHandler()) + + +def add_thing_log_destination( + invocation_id: UUID, destination: MutableSequence +) -> None: + """Append logs matching ``invocation_id`` to a specified sequence. + + This instructs a handler on the logger used for `.Thing` instances to append a copy + of the logs generated by that invocation to the specified sequence. + This is primarily used by invocation threads to collect their logs, so they + may be returned when the invocation is queried. + + :param invocation_id: the ``invocation_id`` to match. + :param destination: should specify a deque, to which we will append + each log entry as it comes in. This is assumed to be thread + safe. + :raises LogConfigurationError: if there is not exactly one suitable handler. + """ + handlers = [ + h for h in THING_LOGGER.handlers if isinstance(h, DequeByInvocationIDHandler) + ] + if len(handlers) != 1: + if len(handlers) == 0: + msg = "There is no suitable handler on {THING_LOGGER}." + else: + msg = "There were multiple matching handlers on {THING_LOGGER}, " + msg += "which should not happen: this is a LabThings bug." + raise LogConfigurationError(msg) + handler = handlers[0] + handler.add_destination_for_id(invocation_id, destination) diff --git a/src/labthings_fastapi/outputs/mjpeg_stream.py b/src/labthings_fastapi/outputs/mjpeg_stream.py index 2aaf05bd..2c142269 100644 --- a/src/labthings_fastapi/outputs/mjpeg_stream.py +++ b/src/labthings_fastapi/outputs/mjpeg_stream.py @@ -456,8 +456,7 @@ class Camera(lt.Thing): stream = MJPEGStreamDescriptor() - server = lt.ThingServer() - server.add_thing("camera", Camera) + server = lt.ThingServer({"camera": Camera}) :param app: the `fastapi.FastAPI` application to which we are being added. :param thing: the host `.Thing` instance. diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py index c24d3699..3f66fc0e 100644 --- a/src/labthings_fastapi/properties.py +++ b/src/labthings_fastapi/properties.py @@ -58,7 +58,6 @@ class attribute. Documentation is in strings immediately following the TYPE_CHECKING, ) from typing_extensions import Self -import typing from weakref import WeakSet from fastapi import Body, FastAPI @@ -73,10 +72,11 @@ class attribute. Documentation is in strings immediately following the ) from .utilities import labthings_data, wrap_plain_types_in_rootmodel from .utilities.introspection import return_type -from .base_descriptor import BaseDescriptor +from .base_descriptor import FieldTypedBaseDescriptor from .exceptions import ( NotConnectedToServerError, ReadOnlyPropertyError, + MissingTypeError, ) if TYPE_CHECKING: @@ -115,23 +115,6 @@ class MissingDefaultError(ValueError): """ -class InconsistentTypeError(TypeError): - """Different type hints have been given for a property. - - Every property should have a type hint, which may be provided in a few - different ways. If multiple type hints are provided, they must match. - See `.property` for more details. - """ - - -class MissingTypeError(TypeError): - """No type hints have been given for a property. - - Every property should have a type hint, which may be provided in a few - different ways. This error indicates that no type hint was found. - """ - - Value = TypeVar("Value") if TYPE_CHECKING: # It's hard to type check methods, because the type of ``self`` @@ -319,7 +302,7 @@ def property( ) -class BaseProperty(BaseDescriptor[Value], Generic[Value]): +class BaseProperty(FieldTypedBaseDescriptor[Value], Generic[Value]): """A descriptor that marks Properties on Things. This class is used to determine whether an attribute of a `.Thing` should @@ -333,21 +316,9 @@ class BaseProperty(BaseDescriptor[Value], Generic[Value]): def __init__(self) -> None: """Initialise a BaseProperty.""" super().__init__() - self._type: type | None = None self._model: type[BaseModel] | None = None self.readonly: bool = False - @builtins.property - def value_type(self) -> type[Value]: - """The type of this descriptor's value. - - :raises MissingTypeError: if the type has not been set. - :return: the type of the descriptor's value. - """ - if self._type is None: - raise MissingTypeError("This property does not have a valid type.") - return self._type - @builtins.property def model(self) -> type[BaseModel]: """A Pydantic model for the property's type. @@ -530,73 +501,6 @@ def __init__( default=default, default_factory=default_factory ) self.readonly = readonly - self._type: type | None = None # Will be set in __set_name__ - - def __set_name__(self, owner: type[Thing], name: str) -> None: - """Take note of the name and type. - - This function is where we determine the type of the property. It may - be specified in two ways: either by subscripting ``DataProperty`` - or by annotating the attribute: - - .. code-block:: python - - class MyThing(Thing): - subscripted_property = DataProperty[int](0) - annotated_property: int = DataProperty(0) - - The second form often works better with autocompletion, though it is - preferred to use `.property` for consistent naming. - - Neither form allows us to access the type during ``__init__``, which - is why we find the type here. If there is a problem, exceptions raised - will appear to come from the class definition, so it's important to - include the name of the attribute. - - See :ref:`descriptors` for links to the Python docs about when - this function is called. - - :param owner: the `.Thing` subclass to which we are being attached. - :param name: the name to which we have been assigned. - - :raises InconsistentTypeError: if the type is specified twice and - the two types are not identical. - :raises MissingTypeError: if no type hints have been given. - """ - # Call BaseDescriptor so we remember the name - super().__set_name__(owner, name) - - # Check for type subscripts - if hasattr(self, "__orig_class__"): - # We have been instantiated with a subscript, e.g. BaseProperty[int]. - # - # __orig_class__ is set on generic classes when they are instantiated - # with a subscripted type. - self._type = typing.get_args(self.__orig_class__)[0] - - # Check for annotations on the parent class - annotations = typing.get_type_hints(owner, include_extras=True) - field_annotation = annotations.get(name, None) - if field_annotation is not None: - # We have been assigned to an annotated class attribute, e.g. - # myprop: int = BaseProperty(0) - if self._type is not None and self._type != field_annotation: - raise InconsistentTypeError( - f"Property {name} on {owner} has conflicting types.\n\n" - f"The field annotation of {field_annotation} conflicts " - f"with the inferred type of {self._type}." - ) - self._type = field_annotation - if self._type is None: - raise MissingTypeError( - f"No type hint was found for property {name} on {owner}." - ) - - @builtins.property - def value_type(self) -> type[Value]: # noqa: DOC201 - """The type of the descriptor's value.""" - self.assert_set_name_called() - return super().value_type def instance_get(self, obj: Thing) -> Value: """Return the property's value. @@ -651,6 +555,9 @@ def emit_changed_event(self, obj: Thing, value: Value) -> None: thread as it is communicating with the event loop via an `asyncio` blocking portal and can cause deadlock if run in the event loop. + This method will raise a `.ServerNotRunningError` if the event loop is not + running, and should only be called after the server has started. + :param obj: the `.Thing` to which we are attached. :param value: the new property value, to be sent to observers. """ @@ -697,10 +604,18 @@ def __init__( tools understand that it functions like a property. :param fget: the getter function, called when the property is read. + + :raises MissingTypeError: if the getter does not have a return type annotation. """ super().__init__() self._fget: ValueGetter = fget self._type = return_type(self._fget) + if self._type is None: + msg = ( + f"{fget} does not have a valid type. " + "Return type annotations are required for property getters." + ) + raise MissingTypeError(msg) self._fset: ValueSetter | None = None self.readonly: bool = True diff --git a/src/labthings_fastapi/server/__init__.py b/src/labthings_fastapi/server/__init__.py index 8efc3967..89ed16e9 100644 --- a/src/labthings_fastapi/server/__init__.py +++ b/src/labthings_fastapi/server/__init__.py @@ -7,34 +7,35 @@ """ from __future__ import annotations -from typing import Any, AsyncGenerator, Optional, Sequence, TypeVar -import os.path -import re +from typing import AsyncGenerator, Optional, TypeVar +from typing_extensions import Self +import os from fastapi import FastAPI, Request from fastapi.middleware.cors import CORSMiddleware from anyio.from_thread import BlockingPortal from contextlib import asynccontextmanager, AsyncExitStack -from collections.abc import Mapping +from collections.abc import Mapping, Sequence from types import MappingProxyType -from ..utilities.object_reference_to_object import ( - object_reference_to_object, -) +from ..thing_slots import ThingSlot +from ..utilities import class_attributes + from ..actions import ActionManager +from ..logs import configure_thing_logger from ..thing import Thing from ..thing_server_interface import ThingServerInterface from ..thing_description._model import ThingDescription from ..dependencies.thing_server import _thing_servers # noqa: F401 +from .config_model import ( + ThingsConfig, + ThingServerConfig, + normalise_things_config as normalise_things_config, +) # `_thing_servers` is used as a global from `ThingServer.__init__` from ..outputs.blob import BlobDataManager -# A path should be made up of names separated by / as a path separator. -# Each name should be made of alphanumeric characters, hyphen, or underscore. -# This regex enforces a trailing / -PATH_REGEX = re.compile(r"^([a-zA-Z0-9\-_]+)$") - ThingSubclass = TypeVar("ThingSubclass", bound=Thing) @@ -57,8 +58,12 @@ class ThingServer: an `anyio.from_thread.BlockingPortal`. """ - def __init__(self, settings_folder: Optional[str] = None) -> None: - """Initialise a LabThings server. + def __init__( + self, + things: ThingsConfig, + settings_folder: Optional[str] = None, + ) -> None: + r"""Initialise a LabThings server. Setting up the `.ThingServer` involves creating the underlying `fastapi.FastAPI` app, setting its lifespan function (used to @@ -68,28 +73,43 @@ def __init__(self, settings_folder: Optional[str] = None) -> None: We also create the `.ActionManager` to manage :ref:`actions` and the `.BlobManager` to manage the downloading of :ref:`blobs`. + :param things: A mapping of Thing names to `.Thing` subclasses, or + `.ThingConfig` objects specifying the subclass, its initialisation + arguments, and any connections to other `.Thing`\ s. :param settings_folder: the location on disk where `.Thing` settings will be saved. """ + configure_thing_logger() # Note: this is safe to call multiple times. + self._config = ThingServerConfig(things=things, settings_folder=settings_folder) self.app = FastAPI(lifespan=self.lifespan) - self.set_cors_middleware() + self._set_cors_middleware() self.settings_folder = settings_folder or "./settings" self.action_manager = ActionManager() self.action_manager.attach_to_app(self.app) self.blob_data_manager = BlobDataManager() self.blob_data_manager.attach_to_app(self.app) - self.add_things_view_to_app() - self._things: dict[str, Thing] = {} + self._add_things_view_to_app() self.blocking_portal: Optional[BlockingPortal] = None self.startup_status: dict[str, str | dict] = {"things": {}} global _thing_servers # noqa: F824 _thing_servers.add(self) + # The function calls below create and set up the Things. + self._things = self._create_things() + self._connect_things() + self._attach_things_to_server() - app: FastAPI - action_manager: ActionManager - blob_data_manager: BlobDataManager + @classmethod + def from_config(cls, config: ThingServerConfig) -> Self: + r"""Create a ThingServer from a configuration model. - def set_cors_middleware(self) -> None: + This is equivalent to ``ThingServer(**dict(config))``\ . + + :param config: The configuration parameters for the server. + :return: A `.ThingServer` configured as per the model. + """ + return cls(**dict(config)) + + def _set_cors_middleware(self) -> None: """Configure the server to allow requests from other origins. This is required to allow web applications access to the HTTP API, @@ -147,75 +167,10 @@ def thing_by_class(self, cls: type[ThingInstance]) -> ThingInstance: f"There are {len(instances)} Things of class {cls}, expected 1." ) - def add_thing( - self, - name: str, - thing_subclass: type[ThingSubclass], - args: Sequence[Any] | None = None, - kwargs: Mapping[str, Any] | None = None, - ) -> ThingSubclass: - r"""Add a thing to the server. - - This function will create an instance of ``thing_subclass`` and supply - the ``args`` and ``kwargs`` arguments to its ``__init__`` method. That - instance will then be added to the server with the given name. - - :param name: The name to use for the thing. This will be part of the URL - used to access the thing, and must only contain alphanumeric characters, - hyphens and underscores. - :param thing_subclass: The `.Thing` subclass to add to the server. - :param args: positional arguments to pass to the constructor of - ``thing_subclass``\ . - :param kwargs: keyword arguments to pass to the constructor of - ``thing_subclass``\ . - - :returns: the instance of ``thing_subclass`` that was created and added - to the server. There is no need to retain a reference to this, as it - is stored in the server's dictionary of `.Thing` instances. - - :raise ValueError: if ``path`` contains invalid characters. - :raise KeyError: if a `.Thing` has already been added at ``path``\ . - :raise TypeError: if ``thing_subclass`` is not a subclass of `.Thing` - or if ``name`` is not string-like. This usually means arguments - are being passed the wrong way round. - """ - if not isinstance(name, str): - raise TypeError("Thing names must be strings.") - if PATH_REGEX.match(name) is None: - msg = ( - f"'{name}' contains unsafe characters. Use only alphanumeric " - "characters, hyphens and underscores" - ) - raise ValueError(msg) - if name in self._things: - raise KeyError(f"{name} has already been added to this thing server.") - if not issubclass(thing_subclass, Thing): - raise TypeError(f"{thing_subclass} is not a Thing subclass.") - if args is None: - args = [] - if kwargs is None: - kwargs = {} - interface = ThingServerInterface(name=name, server=self) - os.makedirs(interface.settings_folder, exist_ok=True) - # This is where we instantiate the Thing - # I've had to ignore this line because the *args causes an error. - # Given that *args and **kwargs are very loosely typed anyway, this - # doesn't lose us much. - thing = thing_subclass( - *args, - **kwargs, - thing_server_interface=interface, - ) # type: ignore[misc] - self._things[name] = thing - thing.attach_to_server( - server=self, - ) - return thing - def path_for_thing(self, name: str) -> str: """Return the path for a thing with the given name. - :param name: The name of the thing, as passed to `.add_thing`. + :param name: The name of the thing. :return: The path at which the thing is served. @@ -225,6 +180,65 @@ def path_for_thing(self, name: str) -> str: raise KeyError(f"No thing named {name} has been added to this server.") return f"/{name}/" + def _create_things(self) -> Mapping[str, Thing]: + r"""Create the Things, add them to the server, and connect them up if needed. + + This method is responsible for creating instances of `.Thing` subclasses + and adding them to the server. It also ensures the `.Thing`\ s are connected + together if required. + + The Things are defined in ``self._config.thing_configs`` which in turn is + generated from the ``things`` argument to ``__init__``\ . + + :return: A mapping of names to `.Thing` instances. + + :raise TypeError: if ``cls`` is not a subclass of `.Thing`. + """ + things: dict[str, Thing] = {} + for name, config in self._config.thing_configs.items(): + if not issubclass(config.cls, Thing): + raise TypeError(f"{config.cls} is not a Thing subclass.") + interface = ThingServerInterface(name=name, server=self) + os.makedirs(interface.settings_folder, exist_ok=True) + # This is where we instantiate the Thing + things[name] = config.cls( + *config.args, + **config.kwargs, + thing_server_interface=interface, + ) + return things + + def _connect_things(self) -> None: + r"""Connect the `thing_slot` attributes of Things. + + A `.Thing` may have attributes defined as ``lt.thing_slot()``, which + will be populated after all `.Thing` instances are loaded on the server. + + This function is responsible for supplying the `.Thing` instances required + for each connection. This will be done by using the name specified either + in the connection's default, or in the configuration of the server. + + `.ThingSlotError` will be raised by code called by this method if + the connection cannot be provided. See `.ThingSlot.connect` for more + details. + """ + for thing_name, thing in self.things.items(): + config = self._config.thing_configs[thing_name].thing_slots + for attr_name, attr in class_attributes(thing): + if not isinstance(attr, ThingSlot): + continue + target = config.get(attr_name, ...) + attr.connect(thing, self.things, target) + + def _attach_things_to_server(self) -> None: + """Add the Things to the FastAPI App. + + This calls `.Thing.attach_to_server` on each `.Thing` that is a part of + this `.ThingServer` in order to add the HTTP endpoints and load settings. + """ + for thing in self.things.values(): + thing.attach_to_server(self) + @asynccontextmanager async def lifespan(self, app: FastAPI) -> AsyncGenerator[None]: """Manage set up and tear down of the server and Things. @@ -249,6 +263,7 @@ async def lifespan(self, app: FastAPI) -> AsyncGenerator[None]: # We create a blocking portal to allow threaded code to call async code # in the event loop. self.blocking_portal = portal + # we __aenter__ and __aexit__ each Thing, which will in turn call the # synchronous __enter__ and __exit__ methods if they exist, to initialise # and shut down the hardware. NB we must make sure the blocking portal @@ -260,7 +275,7 @@ async def lifespan(self, app: FastAPI) -> AsyncGenerator[None]: self.blocking_portal = None - def add_things_view_to_app(self) -> None: + def _add_things_view_to_app(self) -> None: """Add an endpoint that shows the list of attached things.""" thing_server = self @@ -302,38 +317,3 @@ def thing_paths(request: Request) -> Mapping[str, str]: t: f"{str(request.base_url).rstrip('/')}{t}" for t in thing_server.things.keys() } - - -def server_from_config(config: dict) -> ThingServer: - r"""Create a ThingServer from a configuration dictionary. - - This function creates a `.ThingServer` and adds a number of `.Thing` - instances from a configuration dictionary. - - :param config: A dictionary, in the format used by :ref:`config_files` - - :return: A `.ThingServer` with instances of the specified `.Thing` - subclasses attached. The server will not be started by this - function. - - :raise ImportError: if a Thing could not be loaded from the specified - object reference. - """ - server = ThingServer(config.get("settings_folder", None)) - for name, thing in config.get("things", {}).items(): - if isinstance(thing, str): - thing = {"class": thing} - try: - cls = object_reference_to_object(thing["class"]) - except ImportError as e: - raise ImportError( - f"Could not import {thing['class']}, which was " - f"specified as the class for {name}." - ) from e - server.add_thing( - name=name, - thing_subclass=cls, - args=thing.get("args", ()), - kwargs=thing.get("kwargs", {}), - ) - return server diff --git a/src/labthings_fastapi/server/cli.py b/src/labthings_fastapi/server/cli.py index 2e636d28..5af8c01e 100644 --- a/src/labthings_fastapi/server/cli.py +++ b/src/labthings_fastapi/server/cli.py @@ -19,15 +19,15 @@ """ from argparse import ArgumentParser, Namespace +import sys from typing import Optional -import json -from ..utilities.object_reference_to_object import ( - object_reference_to_object, -) +from pydantic import ValidationError import uvicorn -from . import ThingServer, server_from_config +from . import ThingServer +from . import fallback +from .config_model import ThingServerConfig def get_default_parser() -> ArgumentParser: @@ -74,7 +74,7 @@ def parse_args(argv: Optional[list[str]] = None) -> Namespace: return parser.parse_args(argv) -def config_from_args(args: Namespace) -> dict: +def config_from_args(args: Namespace) -> ThingServerConfig: """Load the configuration from a supplied file or JSON string. This function will first attempt to load a JSON file specified in the @@ -87,29 +87,26 @@ def config_from_args(args: Namespace) -> dict: :param args: Parsed arguments from `.parse_args`. - :return: a server configuration, as a dictionary. + :return: the server configuration. :raise FileNotFoundError: if the configuration file specified is missing. :raise RuntimeError: if neither a config file nor a string is provided. """ if args.config: + if args.json: + raise RuntimeError("Can't use both --config and --json simultaneously.") try: with open(args.config) as f: - config = json.load(f) + return ThingServerConfig.model_validate_json(f.read()) except FileNotFoundError as e: raise FileNotFoundError( f"Could not find configuration file {args.config}" ) from e + elif args.json: + return ThingServerConfig.model_validate_json(args.json) else: - config = {} - if args.json: - config.update(json.loads(args.json)) - - if len(config) == 0: raise RuntimeError("No configuration (or empty configuration) provided") - return config - def serve_from_cli( argv: Optional[list[str]] = None, dry_run: bool = False @@ -118,7 +115,7 @@ def serve_from_cli( This function will parse command line arguments, load configuration, set up a server, and start it. It calls `.parse_args`, - `.config_from_args` and `.server_from_config` to get a server, then + `.config_from_args` and `.ThingServer.from_config` to get a server, then starts `uvicorn` to serve on the specified host and port. If the ``fallback`` argument is specified, errors that stop the @@ -127,6 +124,8 @@ def serve_from_cli( if ``labthings-server`` is being run on a headless server, where an HTTP error page is more useful than no response. + If ``fallback`` is not specified, we will print the error and exit. + :param argv: command line arguments (defaults to arguments supplied to the current command). :param dry_run: may be set to ``True`` to terminate after the server @@ -143,20 +142,23 @@ def serve_from_cli( try: config, server = None, None config = config_from_args(args) - server = server_from_config(config) + server = ThingServer.from_config(config) if dry_run: return server uvicorn.run(server.app, host=args.host, port=args.port) except BaseException as e: if args.fallback: print(f"Error: {e}") - fallback_server = "labthings_fastapi.server.fallback:app" - print(f"Starting fallback server {fallback_server}.") - app = object_reference_to_object(fallback_server) + print("Starting fallback server.") + app = fallback.app app.labthings_config = config app.labthings_server = server app.labthings_error = e uvicorn.run(app, host=args.host, port=args.port) else: - raise e + if isinstance(e, ValidationError): + print(f"Error reading LabThings configuration:\n{e}") + sys.exit(3) + else: + raise e return None # This is required as we sometimes return the server diff --git a/src/labthings_fastapi/server/config_model.py b/src/labthings_fastapi/server/config_model.py new file mode 100644 index 00000000..38e4ca8f --- /dev/null +++ b/src/labthings_fastapi/server/config_model.py @@ -0,0 +1,140 @@ +r"""Pydantic models to enable server configuration to be loaded from file. + +The models in this module allow `.ThingConfig` dataclasses to be constructed +from dictionaries or JSON files. They also describe the full server configuration +with `.ServerConfigModel`\ . These models are used by the `.cli` module to +start servers based on configuration files or strings. +""" + +from pydantic import BaseModel, Field, ImportString, AliasChoices, field_validator +from typing import Any, Annotated, TypeAlias +from collections.abc import Mapping, Sequence, Iterable + + +# The type: ignore below is a spurious warning about `kwargs`. +# see https://github.com/pydantic/pydantic/issues/3125 +class ThingConfig(BaseModel): # type: ignore[no-redef] + r"""The information needed to add a `.Thing` to a `.ThingServer`\ .""" + + cls: ImportString = Field( + validation_alias=AliasChoices("cls", "class"), + description="The Thing subclass to add to the server.", + ) + + args: Sequence[Any] = Field( + default_factory=list, + description="Positional arguments to pass to the constructor of `cls`.", + ) + + kwargs: Mapping[str, Any] = Field( + default_factory=dict, + description="Keyword arguments to pass to the constructor of `cls`.", + ) + + thing_slots: Mapping[str, str | Iterable[str] | None] = Field( + default_factory=dict, + description=( + """Connections to other Things. + + Keys are the names of attributes of the Thing and the values are + the name(s) of the Thing(s) you'd like to connect. If this is left + at its default, the connections will use their default behaviour, usually + automatically connecting to a Thing of the right type. + """ + ), + ) + + +ThingName = Annotated[ + str, + Field(min_length=1, pattern=r"^([a-zA-Z0-9\-_]+)$"), +] + + +ThingsConfig: TypeAlias = Mapping[ThingName, ThingConfig | ImportString] + + +class ThingServerConfig(BaseModel): + r"""The configuration parameters for a `.ThingServer`\ .""" + + things: ThingsConfig = Field( + description=( + """A mapping of names to Thing configurations. + + Each Thing on the server must be given a name, which is the dictionary + key. The value is either the class to be used, or a `.ThingConfig` + object specifying the class, initial arguments, and other settings. + """ + ), + ) + + @field_validator("things", mode="after") + @classmethod + def check_things(cls, things: ThingsConfig) -> ThingsConfig: + """Check that the thing configurations can be normalised. + + It's possible to specify the things as a mapping from names to classes. + We use `pydantic.ImportString` as the type of the classes: this takes a + string, and imports the corresponding Python object. When loading config + from JSON, this does the right thing - but when loading from Python objects + it will accept any Python object. + + This validator runs `.normalise_thing_config` to check each value is either + a valid `.ThingConfig` or a type or a mapping. If it's a mapping, we + will attempt to make a `.ThingConfig` from it. If it's a `type` we will + create a `.ThingConfig` using that type as the class. We don't check for + `.Thing` subclasses in this module to avoid a dependency loop. + + :param things: The validated value of the field. + + :return: A copy of the input, with all values converted to `.ThingConfig` + instances. + """ + return normalise_things_config(things) + + @property + def thing_configs(self) -> Mapping[ThingName, ThingConfig]: + r"""A copy of the ``things`` field where every value is a ``.ThingConfig``\ . + + The field validator on ``things`` already ensures it returns a mapping, but + it's not typed strictly, to allow Things to be specified with just a class. + + This property returns the list of `.ThingConfig` objects, and is typed strictly. + """ + return normalise_things_config(self.things) + + settings_folder: str | None = Field( + default=None, + description="The location of the settings folder.", + ) + + +def normalise_things_config(things: ThingsConfig) -> Mapping[ThingName, ThingConfig]: + r"""Ensure every Thing is defined by a `.ThingConfig` object. + + Things may be specified either using a `.ThingConfig` object, or just a bare + `.Thing` subclass, if the other parameters are not needed. To simplify code that + uses the configuration, this function wraps bare classes in a `.ThingConfig` so + the values are uniformly typed. + + :param things: A mapping of names to Things, either classes or `.ThingConfig` + objects. + + :return: A mapping of names to `.ThingConfig` objects. + + :raises ValueError: if a Python object is passed that's neither a `type` nor + a `dict`\ . + """ + normalised: dict[str, ThingConfig] = {} + for k, v in things.items(): + if isinstance(v, ThingConfig): + normalised[k] = v + elif isinstance(v, Mapping): + normalised[k] = ThingConfig.model_validate(v) + elif isinstance(v, type): + normalised[k] = ThingConfig(cls=v) + else: + raise ValueError( + "Things must be specified either as a class or a ThingConfig." + ) + return normalised diff --git a/src/labthings_fastapi/server/fallback.py b/src/labthings_fastapi/server/fallback.py index 388a9f96..399656ed 100644 --- a/src/labthings_fastapi/server/fallback.py +++ b/src/labthings_fastapi/server/fallback.py @@ -9,11 +9,15 @@ import json from traceback import format_exception -from typing import Any +from typing import Any, TYPE_CHECKING from fastapi import FastAPI from fastapi.responses import HTMLResponse from starlette.responses import RedirectResponse +if TYPE_CHECKING: + from . import ThingServer + from .config_model import ThingServerConfig + class FallbackApp(FastAPI): """A basic FastAPI application to serve a LabThings error page.""" @@ -28,9 +32,9 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: :param \**kwargs: is passed to `fastapi.FastAPI.__init__`\ . """ super().__init__(*args, **kwargs) - self.labthings_config = None - self.labthings_server = None - self.labthings_error = None + self.labthings_config: ThingServerConfig | None = None + self.labthings_server: ThingServer | None = None + self.labthings_error: BaseException | None = None self.log_history = None self.html_code = 500 diff --git a/src/labthings_fastapi/thing.py b/src/labthings_fastapi/thing.py index fd49fcdc..9935a3c3 100644 --- a/src/labthings_fastapi/thing.py +++ b/src/labthings_fastapi/thing.py @@ -20,6 +20,7 @@ from pydantic import BaseModel +from .logs import THING_LOGGER from .properties import BaseProperty, DataProperty, BaseSetting from .descriptors import ActionDescriptor from .thing_description._model import ThingDescription, NoSecurityScheme @@ -101,6 +102,16 @@ def path(self) -> str: """The path at which the `.Thing` is exposed over HTTP.""" return self._thing_server_interface.path + @property + def name(self) -> str: + """The name of this Thing, as known to the server.""" + return self._thing_server_interface.name + + @property + def logger(self) -> logging.Logger: + """A logger, named after this Thing.""" + return THING_LOGGER.getChild(self.name) + async def __aenter__(self) -> Self: """Context management is used to set up/close the thing. diff --git a/src/labthings_fastapi/thing_server_interface.py b/src/labthings_fastapi/thing_server_interface.py index c11116f6..85f52ff3 100644 --- a/src/labthings_fastapi/thing_server_interface.py +++ b/src/labthings_fastapi/thing_server_interface.py @@ -138,17 +138,31 @@ def get_thing_states(self) -> Mapping[str, Any]: class MockThingServerInterface(ThingServerInterface): - """A mock class that simulates a ThingServerInterface without the server.""" + r"""A mock class that simulates a ThingServerInterface without the server. - def __init__(self, name: str) -> None: + This allows a `.Thing` to be instantiated but not connected to a server. + The methods normally provided by the server are mocked, specifically: + + * The `name` is set by an argument to `__init__`\ . + * `start_async_task_soon` silently does nothing, i.e. the async function + will not be run. + * The settings folder will either be specified when the class is initialised, + or a temporary folder will be created. + * `get_thing_states` will return an empty dictionary. + """ + + def __init__(self, name: str, settings_folder: str | None = None) -> None: """Initialise a ThingServerInterface. :param name: The name of the Thing we're providing an interface to. + :param settings_folder: The location where we should save settings. + By default, this is a temporary directory. """ # We deliberately don't call super().__init__(), as it won't work without # a server. self._name: str = name self._settings_tempdir: TemporaryDirectory | None = None + self._settings_folder = settings_folder def start_async_task_soon( self, async_function: Callable[Params, Awaitable[ReturnType]], *args: Any @@ -183,6 +197,8 @@ def settings_folder(self) -> str: :returns: the path to a temporary folder. """ + if self._settings_folder: + return self._settings_folder if not self._settings_tempdir: self._settings_tempdir = TemporaryDirectory() return self._settings_tempdir.name @@ -208,7 +224,10 @@ def get_thing_states(self) -> Mapping[str, Any]: def create_thing_without_server( - cls: type[ThingSubclass], *args: Any, **kwargs: Any + cls: type[ThingSubclass], + *args: Any, + settings_folder: str | None = None, + **kwargs: Any, ) -> ThingSubclass: r"""Create a `.Thing` and supply a mock ThingServerInterface. @@ -220,6 +239,8 @@ def create_thing_without_server( :param cls: The `.Thing` subclass to instantiate. :param \*args: positional arguments to ``__init__``. + :param settings_folder: The path to the settings folder. A temporary folder + is used by default. :param \**kwargs: keyword arguments to ``__init__``. :returns: an instance of ``cls`` with a `.MockThingServerInterface` @@ -233,7 +254,11 @@ def create_thing_without_server( msg = "You may not supply a keyword argument called 'thing_server_interface'." raise ValueError(msg) return cls( - *args, **kwargs, thing_server_interface=MockThingServerInterface(name=name) + *args, + **kwargs, + thing_server_interface=MockThingServerInterface( + name=name, settings_folder=settings_folder + ), ) # type: ignore[misc] # Note: we must ignore misc typing errors above because mypy flags an error # that `thing_server_interface` is multiply specified. diff --git a/src/labthings_fastapi/thing_slots.py b/src/labthings_fastapi/thing_slots.py new file mode 100644 index 00000000..25a5b2de --- /dev/null +++ b/src/labthings_fastapi/thing_slots.py @@ -0,0 +1,451 @@ +r"""Facilitate connections between Things. + +It is often desirable for two Things in the same server to be able to communicate. +In order to do this in a nicely typed way that is easy to test and inspect, +LabThings-FastAPI provides the `.thing_slot`\ . This allows a `.Thing` +to declare that it depends on another `.Thing` being present, and provides a way for +the server to automatically connect the two when the server is set up. + +Thing connections are set up **after** all the `.Thing` instances are initialised. +This means you should not rely on them during initialisation: if you attempt to +access a connection before it is provided, it will raise an exception. The +advantage of making connections after initialisation is that circular connections +are not a problem: Thing `a` may depend on Thing `b` and vice versa. + +As with properties, thing connections will usually be declared using the function +`.thing_slot` rather than the descriptor directly. This allows them to be +typed and documented on the class, i.e. + +.. code-block:: python + + import labthings_fastapi as lt + + + class ThingA(lt.Thing): + "A class that doesn't do much." + + @lt.action + def say_hello(self) -> str: + "A canonical example function." + return "Hello world." + + + class ThingB(lt.Thing): + "A class that relies on ThingA." + + thing_a: ThingA = lt.thing_slot() + + @lt.action + def say_hello(self) -> str: + "I'm too lazy to say hello, ThingA does it for me." + return self.thing_a.say_hello() +""" + +from types import EllipsisType, NoneType, UnionType +from typing import Any, Generic, TypeVar, TYPE_CHECKING, Union, get_args, get_origin +from collections.abc import Mapping, Iterable, Sequence +from weakref import ReferenceType, WeakKeyDictionary, ref, WeakValueDictionary +from .base_descriptor import FieldTypedBaseDescriptor +from .exceptions import ThingNotConnectedError, ThingSlotError + +if TYPE_CHECKING: + from .thing import Thing + + +ThingSubclass = TypeVar("ThingSubclass", bound="Thing") +ConnectedThings = TypeVar( + "ConnectedThings", + bound="Mapping[str, Thing] | Thing | None", +) + + +class ThingSlot(Generic[ConnectedThings], FieldTypedBaseDescriptor[ConnectedThings]): + r"""Descriptor that instructs the server to supply other Things. + + A `.ThingSlot` provides either one or several + `.Thing` instances as a property of a `.Thing`\ . This allows `.Thing`\ s + to communicate with each other within the server, including accessing + attributes that are not exposed over HTTP. + + While it is possible to dynamically retrieve a `.Thing` from the `.ThingServer` + this is not recommended: using Thing Connections ensures all the `.Thing` + instances are available before the server starts, reducing the likelihood + of run-time crashes. + + The usual way of creating these connections is the function + `.thing_slot`\ . This class and its subclasses are not usually + instantiated directly. + + The type of the `.ThingSlot` attribute is key to its operation. + It should be assigned to an attribute typed either as a `.Thing` subclass, + a mapping of strings to `.Thing` or subclass instances, or an optional + `.Thing` instance: + + .. code-block:: python + + class OtherExample(lt.Thing): + pass + + + class Example(lt.Thing): + # This will always evaluate to an `OtherExample` + other_thing: OtherExample = lt.thing_slot("other_thing") + + # This may evaluate to an `OtherExample` or `None` + optional: OtherExample | None = lt.thing_slot("other_thing") + + # This evaluates to a mapping of `str` to `.Thing` instances + things: Mapping[str, OtherExample] = lt.thing_slot(["thing_a"]) + """ + + def __init__( + self, *, default: str | None | Iterable[str] | EllipsisType = ... + ) -> None: + """Declare a ThingSlot. + + :param default: The name of the Thing(s) that will be connected by default. + + If the type is optional (e.g. ``ThingSubclass | None``) a default + value of ``None`` will result in the connection evaluating to ``None`` + unless it has been configured by the server. + + If the type is not optional, a default value of ``None`` will result + in an error, unless the server has set another value in its + configuration. + + If the type is a mapping of `str` to `.Thing` the default should be + of type `Iterable[str]` (and could be an empty list). + """ + super().__init__() + self._default = default + self._things: WeakKeyDictionary[ + "Thing", ReferenceType["Thing"] | WeakValueDictionary[str, "Thing"] | None + ] = WeakKeyDictionary() + + @property + def thing_type(self) -> tuple[type, ...]: + r"""The `.Thing` subclass(es) returned by this connection. + + A tuple is returned to allow for optional thing connections that + are typed as the union of two Thing types. It will work with + `isinstance`\ . + """ + thing_type = self.value_type + if self.is_mapping: + # is_mapping already checks the type is a `Mapping`, so + # we can just look at its arguments. + _, thing_type = get_args(self.value_type) + if get_origin(thing_type) in (UnionType, Union): + # If it's a Union, we may have an optional type, in which + # case we want to exclude None. + return tuple(t for t in get_args(thing_type) if t is not NoneType) + else: + # If it's not a Union, it should be a single Thing subclass + # so wrap it in a tuple. + return (thing_type,) + + @property + def is_mapping(self) -> bool: + """Whether we return a mapping of strings to Things, or a single Thing.""" + return get_origin(self.value_type) is Mapping + + @property + def is_optional(self) -> bool: + """Whether ``None`` or an empty mapping is an allowed value.""" + if get_origin(self.value_type) in (UnionType, Union): + if NoneType in get_args(self.value_type): + return True + return False + + @property + def default(self) -> str | Iterable[str] | None | EllipsisType: + """The name of the Thing that will be connected by default, if any.""" + return self._default + + def __set__(self, obj: "Thing", value: ThingSubclass) -> None: + """Raise an error as this is a read-only descriptor. + + :param obj: the `.Thing` on which the descriptor is defined. + :param value: the value being assigned. + + :raises AttributeError: this descriptor is not writeable. + """ + raise AttributeError("This descriptor is read-only.") + + def _pick_things( + self, + things: "Mapping[str, Thing]", + target: str | Iterable[str] | None | EllipsisType, + ) -> "Sequence[Thing]": + r"""Pick the Things we should connect to from a list. + + This function is used internally by `.ThingSlot.connect` to choose + the Things we return when the `.ThingSlot` is accessed. + + :param things: the available `.Thing` instances on the server. + :param target: the name(s) we should connect to, or `None` to set the + connection to `None` (if it is optional). A special value is `...` + which will pick the `.Thing` instannce(s) matching this connection's + type hint. + + :raises ThingSlotError: if the supplied `.Thing` is of the wrong + type, if a sequence is supplied when a single `.Thing` is required, + or if `None` is supplied and the connection is not optional. + :raises TypeError: if ``target`` is not one of the allowed types. + + `KeyError` will also be raised if names specified in ``target`` do not + exist in ``things``\ . + + :return: a list of `.Thing` instances to supply in response to ``__get__``\ . + """ + if target is None: + return [] + elif target is ...: + return [ + thing + for _, thing in things.items() + if isinstance(thing, self.thing_type) + ] + elif isinstance(target, str): + if not isinstance(things[target], self.thing_type): + raise ThingSlotError(f"{target} is the wrong type") + return [things[target]] + elif isinstance(target, Iterable): + for t in target: + if not isinstance(things[t], self.thing_type): + raise ThingSlotError(f"{t} is the wrong type") + return [things[t] for t in target] + msg = f"The target specified for a ThingSlot ({target}) has the wrong " + msg += "type. See ThingSlot.connect() docstring for details." + raise TypeError(msg) + + def connect( + self, + host: "Thing", + things: "Mapping[str, Thing]", + target: str | Iterable[str] | None | EllipsisType = ..., + ) -> None: + r"""Find the `.Thing`\ (s) we should supply when accessed. + + This method sets up a ThingSlot on ``host_thing`` by finding the + `.Thing` instance(s) it should supply when its ``__get__`` method is + called. The logic for determining this is: + + * If ``target`` is specified, we look for the specified `.Thing`\ (s). + ``None`` means we should return ``None`` - that's only allowed if the + type hint permits it. + * If ``target`` is not specified or is ``...`` we use the default value + set when the connection was defined. + * If the default value was ``...`` and no target was specified, we will + attempt to find the `.Thing` by type. Most of the time, this is the + desired behaviour. + + If the type of this connection is a ``Mapping``\ , ``target`` should be + a sequence of names. This sequence may be empty. + + ``None`` is treated as equivalent to the empty list, and a list with + one name in it is treated as equivalent to a single name. + + If the type hint of this connection does not permit ``None``\ , and + either ``None`` is specified, or no ``target`` is given and the default + is set as ``None``\ , then an error will be raised. ``None`` will only + be returned at runtime if it is permitted by the type hint. + + :param host: the `.Thing` on which the connection is defined. + :param things: the available `.Thing` instances on the server. + :param target: the name(s) we should connect to, or `None` to set the + connection to `None` (if it is optional). The default is `...` + which will use the default that was set when this `.ThingSlot` + was defined. + + :raises ThingSlotError: if the supplied `.Thing` is of the wrong + type, if a sequence is supplied when a single `.Thing` is required, + or if `None` is supplied and the connection is not optional. + """ + used_target = self.default if target is ... else target + try: + # First, explicitly check for None so we can raise a helpful error. + if used_target is None and not self.is_optional and not self.is_mapping: + raise ThingSlotError("it must be set in configuration") + # Most of the logic is split out into `_pick_things` to separate + # picking the Things from turning them into the correct mapping/reference. + picked = self._pick_things(things, used_target) + if self.is_mapping: + # Mappings may have any number of entries, so no more validation needed. + self._things[host] = WeakValueDictionary({t.name: t for t in picked}) + elif len(picked) == 0: + if self.is_optional: + # Optional things may be set to None without an error. + self._things[host] = None + else: + # Otherwise a single Thing is required, so raise an error. + raise ThingSlotError("no matching Thing was found") + elif len(picked) == 1: + # A single Thing is found: we can safely use this. + self._things[host] = ref(picked[0]) + else: + # If more than one Thing is found (and we're not a mapping) this is + # an error. + raise ThingSlotError("it can't connect to multiple Things") + except (ThingSlotError, KeyError) as e: + reason = str(e.args[0]) + if isinstance(e, KeyError): + reason += " is not the name of a Thing" + msg = f"Can't connect '{host.name}.{self.name}' because {reason}. " + if target is not ...: + msg += f"It was configured to connect to '{target}'. " + else: + msg += "It was not configured, and used the default. " + if self.default is not ...: + msg += f"The default is '{self.default}'." + else: + msg += f"The default searches for Things by type: '{self.thing_type}'." + + raise ThingSlotError(msg) from e + + def instance_get(self, obj: "Thing") -> ConnectedThings: + r"""Supply the connected `.Thing`\ (s). + + :param obj: The `.Thing` on which the connection is defined. + + :return: the `.Thing` instance(s) connected. + + :raises ThingNotConnectedError: if the ThingSlot has not yet been set up. + :raises ReferenceError: if a connected Thing no longer exists (should not + ever happen in normal usage). + + Typing notes: + + This must be annotated as ``ConnectedThings`` which is the type variable + corresponding to the type of this connection. The type determined + at runtime will be within the upper bound of ``ConnectedThings`` but it + would be possible for ``ConnectedThings`` to be more specific. + + In general, types determined at runtime may conflict with generic types, + and at least for this class the important thing is that types determined + at runtime match the attribute annotations, which is tested in unit tests. + + The return statements here consequently have their types ignored. + + """ + msg = f"{self.name} has not been connected to a Thing yet." + try: + val = self._things[obj] + except KeyError as e: + raise ThingNotConnectedError(msg) from e + if isinstance(val, ReferenceType): + thing = val() + if thing is not None: + return thing # type: ignore[return-value] + # See docstring for an explanation of the type ignore directives. + else: + raise ReferenceError("A connected thing was garbage collected.") + else: + # This works for None or for WeakValueDictionary() + return val # type: ignore[return-value] + # See docstring for an explanation of the type ignore directives. + + +def thing_slot(default: str | Iterable[str] | None | EllipsisType = ...) -> Any: + r"""Declare a connection to another `.Thing` in the same server. + + ``lt.thing_slot`` marks a class attribute as a connection to another + `.Thing` on the same server. This will be automatically supplied when the + server is started, based on the type hint and default value. + + In keeping with `.property` and `.setting`, the type of the attribute should + be the type of the connected `.Thing`\ . For example: + + .. code-block:: python + + import labthings_fastapi as lt + + + class ThingA(lt.Thing): ... + + + class ThingB(lt.Thing): + "A class that relies on ThingA." + + thing_a: ThingA = lt.thing_slot() + + This function is a convenience wrapper around the `.ThingSlot` descriptor + class, and should be used in preference to using the descriptor directly. + The main reason to use the function is that it suppresses type errors when + using static type checkers such as `mypy` or `pyright` (see note below). + + The type hint of a Thing Connection should be one of the following: + + * A `.Thing` subclass. An instance of this subclass will be returned when + the attribute is accessed. + * An optional `.Thing` subclass (e.g. ``MyThing | None``). This will either + return a ``MyThing`` instance or ``None``\ . + * A mapping of `str` to `.Thing` (e.g. ``Mapping[str, MyThing]``). This will + return a mapping of `.Thing` names to `.Thing` instances. The mapping + may be empty. + + Example: + + .. code-block:: python + + import labthings_fastapi as lt + + + class ThingA(lt.Thing): + "An example Thing." + + + class ThingB(lt.Thing): + "An example Thing with connections." + + thing_a: ThingA = lt.thing_slot() + maybe_thing_a: ThingA | None = lt.thing_slot() + all_things_a: Mapping[str, ThingA] = lt.thing_slot() + + @lt.thing_action + def show_connections(self) -> str: + "Tell someone about our connections." + self.thing_a # should always evaluate to a ThingA instance + self.maybe_thing_a # will be a ThingA instance or None + self.all_things_a # will a mapping of names to ThingA instances + return f"{self.thing_a=}, {self.maybe_thing_a=}, {self.all_things_a=}" + + The example above is very contrived, but shows how to apply the different types. + + If no default value is supplied, and no value is configured for the connection, + the server will attempt to find a `.Thing` that + matches the specified type when the server is started. If no matching `.Thing` + instances are found, the descriptor will return ``None`` or an empty mapping. + If that is not allowed by the type hint, the server will fail to start with + an error. + + The default value may be a string specifying a `.Thing` name, or a sequence of + strings (for connections that return mappings). In those cases, the relevant + `.Thing` will be returned from the server. If a name is given that either + doesn't correspond to a `.Thing` on the server, or is a `.Thing` that doesn't + match the type of this connection, the server will fail to start with an error. + + The default may also be ``None`` + which is appropriate when the type is optional or a mapping. If the type is + a `.Thing` subclass, a default value of ``None`` forces the connection to be + specified in configuration. + + :param default: The name(s) of the Thing(s) that will be connected by default. + If the default is omitted or set to ``...`` the server will attempt to find + a matching `.Thing` instance (or instances). A default value of `None` is + allowed if the connection is type hinted as optional. + :return: A `.ThingSlot` descriptor. + + Typing notes: + + In the example above, using `.ThingSlot` directly would assign an object + with type ``ThingSlot[ThingA]`` to the attribute ``thing_a``, which is + typed as ``ThingA``\ . This would cause a type error. Using + `.thing_slot` suppresses this error, as its return type is a`Any``\ . + + The use of ``Any`` or an alternative type-checking exemption seems to be + inevitable when implementing descriptors that are typed via attribute annotations, + and it is done by established libraries such as `pydantic`\ . + + """ + return ThingSlot(default=default) diff --git a/src/labthings_fastapi/utilities/object_reference_to_object.py b/src/labthings_fastapi/utilities/object_reference_to_object.py deleted file mode 100644 index 72ddceb3..00000000 --- a/src/labthings_fastapi/utilities/object_reference_to_object.py +++ /dev/null @@ -1,34 +0,0 @@ -"""Load objects from object references.""" - -import importlib -from typing import Any - - -def object_reference_to_object(object_reference: str) -> Any: - """Convert a string reference to an object. - - This is taken from: - https://packaging.python.org/en/latest/specifications/entry-points/ - - The format of the string is `module_name:qualname` where `qualname` - is the fully qualified name of the object within the module. This is - the same format used by entrypoints` in `setup.py` files. - - :param object_reference: a string referencing a Python object to import. - - :return: the Python object. - - :raise ImportError: if the referenced object cannot be found or imported. - """ - modname, qualname_separator, qualname = object_reference.partition(":") - obj = importlib.import_module(modname) - if qualname_separator: - for attr in qualname.split("."): - try: - obj = getattr(obj, attr) - except AttributeError as e: - raise ImportError( - f"Cannot import name {attr} from {obj} " - f"when loading '{object_reference}'" - ) from e - return obj diff --git a/tests/old_dependency_tests/README.md b/tests/old_dependency_tests/README.md new file mode 100644 index 00000000..1752ddfb --- /dev/null +++ b/tests/old_dependency_tests/README.md @@ -0,0 +1,5 @@ +# Old-style dependency tests + +The test files in this folder use the old (pre-v0.0.12) dependency mechanism. This will be removed in v0.0.13, and these tests are preserved here to ensure they work until then. Test files of the same name exist in the parent module, but they have been migrated to use the newer syntax (i.e. not to use dependencies). As of v0.0.13, this folder will be deleted, and the duplication will go away. + +It felt cleaner to duplicate the tests temporarily, rather than try to test two different forms of the syntax in the same file. This way, we keep the old syntax out of the test suite, preserving enough to check it's not broken until it moves from deprecated to gone. diff --git a/tests/old_dependency_tests/__init__.py b/tests/old_dependency_tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/module_with_deps.py b/tests/old_dependency_tests/module_with_deps.py similarity index 100% rename from tests/module_with_deps.py rename to tests/old_dependency_tests/module_with_deps.py diff --git a/tests/old_dependency_tests/test_action_cancel.py b/tests/old_dependency_tests/test_action_cancel.py new file mode 100644 index 00000000..f332ea5e --- /dev/null +++ b/tests/old_dependency_tests/test_action_cancel.py @@ -0,0 +1,203 @@ +""" +This tests that actions may be cancelled. +""" + +import uuid +import pytest +from fastapi.testclient import TestClient +from ..temp_client import poll_task, task_href +import labthings_fastapi as lt +import time + + +class CancellableCountingThing(lt.Thing): + counter: int = lt.property(default=0) + check: bool = lt.property(default=False) + """Whether the count has been cancelled. + + This variable is used to check that the action can detect a cancel event + and react by performing another task, in this case, setting this variable. + """ + + @lt.thing_action + def count_slowly(self, cancel: lt.deps.CancelHook, n: int = 10): + for _i in range(n): + try: + cancel.sleep(0.1) + except lt.exceptions.InvocationCancelledError as e: + # Set check to true to show that cancel was called. + self.check = True + raise (e) + self.counter += 1 + + @lt.thing_action + def count_slowly_but_ignore_cancel(self, cancel: lt.deps.CancelHook, n: int = 10): + """ + Used to check that cancellation alter task behaviour + """ + counting_increment = 1 + for _i in range(n): + try: + cancel.sleep(0.1) + except lt.exceptions.InvocationCancelledError: + # Rather than cancel, this disobedient task just counts faster + counting_increment = 3 + self.counter += counting_increment + + @lt.thing_action + def count_and_only_cancel_if_asked_twice( + self, cancel: lt.deps.CancelHook, n: int = 10 + ): + """ + A task that changes behaviour on cancel, but if asked a second time will cancel + """ + cancelled_once = False + counting_increment = 1 + for _i in range(n): + try: + cancel.sleep(0.1) + except lt.exceptions.InvocationCancelledError as e: + # If this is the second time, this is called actually cancel. + if cancelled_once: + raise (e) + # If not, remember that this cancel event happened. + cancelled_once = True + # Reset the CancelHook + cancel.clear() + # Count backwards instead! + counting_increment = -1 + self.counter += counting_increment + + +@pytest.fixture +def server(): + """Create a server with a CancellableCountingThing added.""" + server = lt.ThingServer({"counting_thing": CancellableCountingThing}) + return server + + +@pytest.fixture +def counting_thing(server): + """Retrieve the CancellableCountingThing from the server.""" + return server.things["counting_thing"] + + +@pytest.fixture +def client(server): + with TestClient(server.app) as client: + yield client + + +def test_invocation_cancel(counting_thing, client): + """ + Test that an invocation can be cancelled and the associated + exception handled correctly. + """ + assert counting_thing.counter == 0 + assert not counting_thing.check + response = client.post("/counting_thing/count_slowly", json={}) + response.raise_for_status() + # Use `client.delete` to cancel the task! + cancel_response = client.delete(task_href(response.json())) + # Raise an exception is this isn't a 2xx response + cancel_response.raise_for_status() + invocation = poll_task(client, response.json()) + assert invocation["status"] == "cancelled" + assert counting_thing.counter < 9 + # Check that error handling worked + assert counting_thing.check + + +def test_invocation_that_refuses_to_cancel(counting_thing, client): + """ + Test that an invocation can detect a cancel request but choose + to modify behaviour. + """ + assert counting_thing.counter == 0 + response = client.post( + "/counting_thing/count_slowly_but_ignore_cancel", json={"n": 5} + ) + response.raise_for_status() + # Use `client.delete` to try to cancel the task! + cancel_response = client.delete(task_href(response.json())) + # Raise an exception is this isn't a 2xx response + cancel_response.raise_for_status() + invocation = poll_task(client, response.json()) + # As the task ignored the cancel. It should return completed + assert invocation["status"] == "completed" + # Counter should be greater than 5 as it counts faster if cancelled! + assert counting_thing.counter > 5 + + +def test_invocation_that_needs_cancel_twice(counting_thing, client): + """ + Test that an invocation can interpret cancel to change behaviour, but + can really cancel if requested a second time + """ + # First cancel only once: + assert counting_thing.counter == 0 + response = client.post( + "/counting_thing/count_and_only_cancel_if_asked_twice", json={"n": 5} + ) + response.raise_for_status() + # Use `client.delete` to try to cancel the task! + cancel_response = client.delete(task_href(response.json())) + # Raise an exception is this isn't a 2xx response + cancel_response.raise_for_status() + invocation = poll_task(client, response.json()) + # As the task ignored the cancel. It should return completed + assert invocation["status"] == "completed" + # Counter should be less than 0 as it should started counting backwards + # almost immediately. + assert counting_thing.counter < 0 + + # Next cancel twice. + counting_thing.counter = 0 + assert counting_thing.counter == 0 + response = client.post( + "/counting_thing/count_and_only_cancel_if_asked_twice", json={"n": 5} + ) + response.raise_for_status() + # Use `client.delete` to try to cancel the task! + cancel_response = client.delete(task_href(response.json())) + # Raise an exception is this isn't a 2xx response + cancel_response.raise_for_status() + # Cancel again + cancel_response2 = client.delete(task_href(response.json())) + # Raise an exception is this isn't a 2xx response + cancel_response2.raise_for_status() + invocation = poll_task(client, response.json()) + # As the task ignored the cancel. It should return completed + assert invocation["status"] == "cancelled" + # Counter should be less than 0 as it should started counting backwards + # almost immediately. + assert counting_thing.counter < 0 + + +def test_late_invocation_cancel_responds_503(counting_thing, client): + """ + Test that cancelling an invocation after it completes returns a 503 response. + """ + assert counting_thing.counter == 0 + assert not counting_thing.check + response = client.post("/counting_thing/count_slowly", json={"n": 1}) + response.raise_for_status() + # Sleep long enough that task completes. + time.sleep(0.3) + poll_task(client, response.json()) + # Use `client.delete` to cancel the task! + cancel_response = client.delete(task_href(response.json())) + # Check a 503 code is returned + assert cancel_response.status_code == 503 + # Check counter reached it's target + assert counting_thing.counter == 1 + # Check that error handling wasn't called + assert not counting_thing.check + + +def test_cancel_unknown_task(counting_thing, client): + """ + Test that cancelling an unknown invocation returns a 404 response + """ + cancel_response = client.delete(f"/invocations/{uuid.uuid4()}") + assert cancel_response.status_code == 404 diff --git a/tests/old_dependency_tests/test_action_logging.py b/tests/old_dependency_tests/test_action_logging.py new file mode 100644 index 00000000..7d1783a4 --- /dev/null +++ b/tests/old_dependency_tests/test_action_logging.py @@ -0,0 +1,127 @@ +""" +This tests the log that is returned in an action invocation +""" + +import logging +from fastapi.testclient import TestClient +import pytest +from ..temp_client import poll_task +import labthings_fastapi as lt +from labthings_fastapi.actions.invocation_model import LogRecordModel +from labthings_fastapi.logs import THING_LOGGER + + +class ThingThatLogsAndErrors(lt.Thing): + LOG_MESSAGES = [ + "message 1", + "message 2", + ] + + @lt.thing_action + def action_that_logs(self, logger: lt.deps.InvocationLogger): + for m in self.LOG_MESSAGES: + logger.info(m) + + @lt.thing_action + def action_with_unhandled_error(self, logger: lt.deps.InvocationLogger): + raise RuntimeError("I was asked to raise this error.") + + @lt.thing_action + def action_with_invocation_error(self, logger: lt.deps.InvocationLogger): + raise lt.exceptions.InvocationError("This is an error, but I handled it!") + + +@pytest.fixture +def client(): + """Set up a Thing Server and yield a client to it.""" + server = lt.ThingServer({"log_and_error_thing": ThingThatLogsAndErrors}) + with TestClient(server.app) as client: + yield client + + +def test_invocation_logging(caplog, client): + """Check the expected items appear in the log when an action is invoked.""" + with caplog.at_level(logging.INFO, logger=THING_LOGGER.name): + r = client.post("/log_and_error_thing/action_that_logs") + r.raise_for_status() + invocation = poll_task(client, r.json()) + assert invocation["status"] == "completed" + assert len(invocation["log"]) == len(ThingThatLogsAndErrors.LOG_MESSAGES) + assert len(invocation["log"]) == len(caplog.records) + for expected, entry in zip( + ThingThatLogsAndErrors.LOG_MESSAGES, invocation["log"], strict=True + ): + assert entry["message"] == expected + + +def test_unhandled_error_logs(caplog, client): + """Check that a log with a traceback is raised if there is an unhandled error.""" + with caplog.at_level(logging.INFO, logger=THING_LOGGER.name): + r = client.post("/log_and_error_thing/action_with_unhandled_error") + r.raise_for_status() + invocation = poll_task(client, r.json()) + assert invocation["status"] == "error" + assert len(invocation["log"]) == len(caplog.records) == 1 + assert caplog.records[0].levelname == "ERROR" + # There is a traceback + assert caplog.records[0].exc_info is not None + + +def test_invocation_error_logs(caplog, client): + """Check that expected errors are logged without a traceback.""" + with caplog.at_level(logging.INFO, logger=THING_LOGGER.name): + r = client.post("/log_and_error_thing/action_with_invocation_error") + r.raise_for_status() + invocation = poll_task(client, r.json()) + assert invocation["status"] == "error" + assert len(invocation["log"]) == len(caplog.records) == 1 + assert caplog.records[0].levelname == "ERROR" + # There is not a traceback + assert caplog.records[0].exc_info is None + + +def test_logrecordmodel(): + record = logging.LogRecord( + name="recordName", + level=logging.INFO, + pathname="dummy/path", + lineno=0, + msg="a string message", + args=None, + exc_info=None, + ) + m = LogRecordModel.model_validate(record, from_attributes=True) + assert m.levelname == record.levelname + + +def test_logrecord_args(): + record = logging.LogRecord( + name="recordName", + level=logging.INFO, + pathname="dummy/path", + lineno=0, + msg="mambo number %d", + args=(5,), + exc_info=None, + ) + m = LogRecordModel.model_validate(record, from_attributes=True) + assert m.message == record.getMessage() + + +def test_logrecord_too_many_args(): + """Calling getMessage() will raise an error - but it should still validate + + If it doesn't validate, it will cause every subsequent call to the action's + invocation record to return a 500 error. + """ + record = logging.LogRecord( + name="recordName", + level=logging.INFO, + pathname="dummy/path", + lineno=0, + msg="mambo number %d", + args=(5, 6), + exc_info=None, + ) + m = LogRecordModel.model_validate(record, from_attributes=True) + assert m.message.startswith("Error") diff --git a/tests/test_dependencies.py b/tests/old_dependency_tests/test_dependencies.py similarity index 100% rename from tests/test_dependencies.py rename to tests/old_dependency_tests/test_dependencies.py diff --git a/tests/test_dependencies_2.py b/tests/old_dependency_tests/test_dependencies_2.py similarity index 100% rename from tests/test_dependencies_2.py rename to tests/old_dependency_tests/test_dependencies_2.py diff --git a/tests/test_dependency_metadata.py b/tests/old_dependency_tests/test_dependency_metadata.py similarity index 64% rename from tests/test_dependency_metadata.py rename to tests/old_dependency_tests/test_dependency_metadata.py index efec5cbf..c1dbac00 100644 --- a/tests/test_dependency_metadata.py +++ b/tests/old_dependency_tests/test_dependency_metadata.py @@ -5,7 +5,7 @@ from typing import Any, Mapping from fastapi.testclient import TestClient import pytest -from .temp_client import poll_task +from ..temp_client import poll_task import labthings_fastapi as lt @@ -38,7 +38,7 @@ def thing_state(self): return {"a": 1} @lt.thing_action - def count_and_watch_deprecated( + def count_and_watch( self, thing_one: ThingOneDep, get_metadata: lt.deps.GetThingStates ) -> Mapping[str, Mapping[str, Any]]: metadata = {} @@ -47,23 +47,16 @@ def count_and_watch_deprecated( metadata[f"a_{a}"] = get_metadata() return metadata - @lt.thing_action - def count_and_watch( - self, thing_one: ThingOneDep - ) -> Mapping[str, Mapping[str, Any]]: - metadata = {} - for a in self.A_VALUES: - thing_one.a = a - metadata[f"a_{a}"] = self._thing_server_interface.get_thing_states() - return metadata - @pytest.fixture def client(): """Yield a test client connected to a ThingServer.""" - server = lt.ThingServer() - server.add_thing("thing_one", ThingOne) - server.add_thing("thing_two", ThingTwo) + server = lt.ThingServer( + { + "thing_one": ThingOne, + "thing_two": ThingTwo, + } + ) with TestClient(server.app) as client: yield client @@ -77,14 +70,3 @@ def test_fresh_metadata(client): for a in ThingTwo.A_VALUES: assert out[f"a_{a}"]["thing_one"]["a"] == a assert out[f"a_{a}"]["thing_two"]["a"] == 1 - - -def test_fresh_metadata_deprecated(client): - """Test that the old metadata dependency retrieves fresh metadata.""" - r = client.post("/thing_two/count_and_watch") - invocation = poll_task(client, r.json()) - assert invocation["status"] == "completed" - out = invocation["output"] - for a in ThingTwo.A_VALUES: - assert out[f"a_{a}"]["thing_one"]["a"] == a - assert out[f"a_{a}"]["thing_two"]["a"] == 1 diff --git a/tests/test_directthingclient.py b/tests/old_dependency_tests/test_directthingclient.py similarity index 96% rename from tests/test_directthingclient.py rename to tests/old_dependency_tests/test_directthingclient.py index 8bf8c8f4..6cc2aa91 100644 --- a/tests/test_directthingclient.py +++ b/tests/old_dependency_tests/test_directthingclient.py @@ -9,7 +9,7 @@ import labthings_fastapi as lt from labthings_fastapi.deps import DirectThingClient, direct_thing_client_class from labthings_fastapi.thing_server_interface import create_thing_without_server -from .temp_client import poll_task +from ..temp_client import poll_task class Counter(lt.Thing): @@ -144,9 +144,12 @@ def test_directthingclient_in_server(action): This uses the internal thing client mechanism. """ - server = lt.ThingServer() - server.add_thing("counter", Counter) - server.add_thing("controller", Controller) + server = lt.ThingServer( + { + "counter": Counter, + "controller": Controller, + } + ) with TestClient(server.app) as client: r = client.post(f"/controller/{action}") invocation = poll_task(client, r.json()) diff --git a/tests/test_thing_dependencies.py b/tests/old_dependency_tests/test_thing_dependencies.py similarity index 92% rename from tests/test_thing_dependencies.py rename to tests/old_dependency_tests/test_thing_dependencies.py index ef7b81bd..ab1b0c8e 100644 --- a/tests/test_thing_dependencies.py +++ b/tests/old_dependency_tests/test_thing_dependencies.py @@ -7,7 +7,7 @@ from fastapi import Request import pytest import labthings_fastapi as lt -from .temp_client import poll_task +from ..temp_client import poll_task from labthings_fastapi.client.in_server import direct_thing_client_class from labthings_fastapi.utilities.introspection import fastapi_dependency_params @@ -80,9 +80,7 @@ def test_interthing_dependency(): This uses the internal thing client mechanism. """ - server = lt.ThingServer() - server.add_thing("thing_one", ThingOne) - server.add_thing("thing_two", ThingTwo) + server = lt.ThingServer({"thing_one": ThingOne, "thing_two": ThingTwo}) with TestClient(server.app) as client: r = client.post("/thing_two/action_two") invocation = poll_task(client, r.json()) @@ -96,10 +94,9 @@ def test_interthing_dependency_with_dependencies(): This uses the internal thing client mechanism, and requires dependency injection for the called action """ - server = lt.ThingServer() - server.add_thing("thing_one", ThingOne) - server.add_thing("thing_two", ThingTwo) - server.add_thing("thing_three", ThingThree) + server = lt.ThingServer( + {"thing_one": ThingOne, "thing_two": ThingTwo, "thing_three": ThingThree} + ) with TestClient(server.app) as client: r = client.post("/thing_three/action_three") r.raise_for_status() @@ -121,9 +118,7 @@ def action_two(self, thing_one: ThingOneDep) -> str: """An action that needs a ThingOne""" return thing_one.action_one() - server = lt.ThingServer() - server.add_thing("thing_one", ThingOne) - server.add_thing("thing_two", ThingTwo) + server = lt.ThingServer({"thing_one": ThingOne, "thing_two": ThingTwo}) with TestClient(server.app) as client: r = client.post("/thing_two/action_two") invocation = poll_task(client, r.json()) diff --git a/tests/test_action_cancel.py b/tests/test_action_cancel.py index 881e6bf1..2713fd8a 100644 --- a/tests/test_action_cancel.py +++ b/tests/test_action_cancel.py @@ -20,10 +20,10 @@ class CancellableCountingThing(lt.Thing): """ @lt.thing_action - def count_slowly(self, cancel: lt.deps.CancelHook, n: int = 10): + def count_slowly(self, n: int = 10): for _i in range(n): try: - cancel.sleep(0.1) + lt.cancellable_sleep(0.1) except lt.exceptions.InvocationCancelledError as e: # Set check to true to show that cancel was called. self.check = True @@ -31,23 +31,21 @@ def count_slowly(self, cancel: lt.deps.CancelHook, n: int = 10): self.counter += 1 @lt.thing_action - def count_slowly_but_ignore_cancel(self, cancel: lt.deps.CancelHook, n: int = 10): + def count_slowly_but_ignore_cancel(self, n: int = 10): """ Used to check that cancellation alter task behaviour """ counting_increment = 1 for _i in range(n): try: - cancel.sleep(0.1) + lt.cancellable_sleep(0.1) except lt.exceptions.InvocationCancelledError: # Rather than cancel, this disobedient task just counts faster counting_increment = 3 self.counter += counting_increment @lt.thing_action - def count_and_only_cancel_if_asked_twice( - self, cancel: lt.deps.CancelHook, n: int = 10 - ): + def count_and_only_cancel_if_asked_twice(self, n: int = 10): """ A task that changes behaviour on cancel, but if asked a second time will cancel """ @@ -55,15 +53,13 @@ def count_and_only_cancel_if_asked_twice( counting_increment = 1 for _i in range(n): try: - cancel.sleep(0.1) + lt.cancellable_sleep(0.1) except lt.exceptions.InvocationCancelledError as e: # If this is the second time, this is called actually cancel. if cancelled_once: raise (e) # If not, remember that this cancel event happened. cancelled_once = True - # Reset the CancelHook - cancel.clear() # Count backwards instead! counting_increment = -1 self.counter += counting_increment @@ -72,8 +68,7 @@ def count_and_only_cancel_if_asked_twice( @pytest.fixture def server(): """Create a server with a CancellableCountingThing added.""" - server = lt.ThingServer() - server.add_thing("counting_thing", CancellableCountingThing) + server = lt.ThingServer({"counting_thing": CancellableCountingThing}) return server diff --git a/tests/test_action_logging.py b/tests/test_action_logging.py index 03b9213a..39749eaa 100644 --- a/tests/test_action_logging.py +++ b/tests/test_action_logging.py @@ -8,6 +8,7 @@ from .temp_client import poll_task import labthings_fastapi as lt from labthings_fastapi.actions.invocation_model import LogRecordModel +from labthings_fastapi.logs import THING_LOGGER class ThingThatLogsAndErrors(lt.Thing): @@ -17,35 +18,35 @@ class ThingThatLogsAndErrors(lt.Thing): ] @lt.thing_action - def action_that_logs(self, logger: lt.deps.InvocationLogger): + def action_that_logs(self): for m in self.LOG_MESSAGES: - logger.info(m) + self.logger.info(m) @lt.thing_action - def action_with_unhandled_error(self, logger: lt.deps.InvocationLogger): + def action_with_unhandled_error(self): raise RuntimeError("I was asked to raise this error.") @lt.thing_action - def action_with_invocation_error(self, logger: lt.deps.InvocationLogger): + def action_with_invocation_error(self): raise lt.exceptions.InvocationError("This is an error, but I handled it!") @pytest.fixture def client(): """Set up a Thing Server and yield a client to it.""" - server = lt.ThingServer() - server.add_thing("log_and_error_thing", ThingThatLogsAndErrors) + server = lt.ThingServer({"log_and_error_thing": ThingThatLogsAndErrors}) with TestClient(server.app) as client: yield client def test_invocation_logging(caplog, client): """Check the expected items appear in the log when an action is invoked.""" - with caplog.at_level(logging.INFO, logger="labthings.action"): + with caplog.at_level(logging.INFO, logger=THING_LOGGER.name): r = client.post("/log_and_error_thing/action_that_logs") r.raise_for_status() invocation = poll_task(client, r.json()) assert invocation["status"] == "completed" + assert len(caplog.records) == len(ThingThatLogsAndErrors.LOG_MESSAGES) assert len(invocation["log"]) == len(ThingThatLogsAndErrors.LOG_MESSAGES) assert len(invocation["log"]) == len(caplog.records) for expected, entry in zip( @@ -56,7 +57,7 @@ def test_invocation_logging(caplog, client): def test_unhandled_error_logs(caplog, client): """Check that a log with a traceback is raised if there is an unhandled error.""" - with caplog.at_level(logging.INFO, logger="labthings.action"): + with caplog.at_level(logging.INFO, logger=THING_LOGGER.name): r = client.post("/log_and_error_thing/action_with_unhandled_error") r.raise_for_status() invocation = poll_task(client, r.json()) @@ -69,7 +70,7 @@ def test_unhandled_error_logs(caplog, client): def test_invocation_error_logs(caplog, client): """Check that a log with a traceback is raised if there is an unhandled error.""" - with caplog.at_level(logging.INFO, logger="labthings.action"): + with caplog.at_level(logging.INFO, logger=THING_LOGGER.name): r = client.post("/log_and_error_thing/action_with_invocation_error") r.raise_for_status() invocation = poll_task(client, r.json()) diff --git a/tests/test_action_manager.py b/tests/test_action_manager.py index 5da65ba5..507806b7 100644 --- a/tests/test_action_manager.py +++ b/tests/test_action_manager.py @@ -28,8 +28,7 @@ def increment_counter_longlife(self): @pytest.fixture def client(): """Yield a TestClient connected to a ThingServer.""" - server = lt.ThingServer() - server.add_thing("thing", CounterThing) + server = lt.ThingServer({"thing": CounterThing}) with TestClient(server.app) as client: yield client diff --git a/tests/test_actions.py b/tests/test_actions.py index 3227087d..5a788b2f 100644 --- a/tests/test_actions.py +++ b/tests/test_actions.py @@ -12,8 +12,7 @@ @pytest.fixture def client(): """Yield a client connected to a ThingServer""" - server = lt.ThingServer() - server.add_thing("thing", MyThing) + server = lt.ThingServer({"thing": MyThing}) with TestClient(server.app) as client: yield client @@ -21,7 +20,8 @@ def client(): def action_partial(client: TestClient, url: str): def run(payload=None): r = client.post(url, json=payload) - assert r.status_code in (200, 201) + if r.status_code not in (200, 201): + raise RuntimeError(f"Received HTTP response code {r.status_code}") return poll_task(client, r.json()) return run @@ -73,6 +73,10 @@ def test_no_args(client): run({}) # an empty dict should be OK run(None) # it should also be OK to call it with None # Calling with no payload is equivalent to None + with pytest.raises(RuntimeError, match="422"): + run(10) # the payload must be a dict - this will error. + with pytest.raises(RuntimeError, match="422"): + run({"key": "value"}) # non-empty dicts should cause an error. def test_only_kwargs(client): @@ -88,6 +92,8 @@ def test_only_kwargs(client): run({}) # an empty dict should be OK run(None) # it should also be OK to call it with None run({"foo": "bar"}) # it should be OK to call it with a payload + with pytest.raises(RuntimeError, match="422"): + run(10) # but the payload must be a dict - this will error. def test_varargs(): diff --git a/tests/test_base_descriptor.py b/tests/test_base_descriptor.py index 34e37637..91b409f1 100644 --- a/tests/test_base_descriptor.py +++ b/tests/test_base_descriptor.py @@ -1,11 +1,14 @@ +import gc import pytest from labthings_fastapi.base_descriptor import ( BaseDescriptor, + FieldTypedBaseDescriptor, DescriptorNotAddedToClassError, DescriptorAddedToClassTwiceError, get_class_attribute_docstrings, ) from .utilities import raises_or_is_caused_by +from labthings_fastapi.exceptions import MissingTypeError, InconsistentTypeError class MockProperty(BaseDescriptor[str]): @@ -284,3 +287,165 @@ class SecondExampleClass: assert "prop" in str(excinfo.value) assert "FirstExampleClass" in str(excinfo.value) assert "SecondExampleClass" in str(excinfo.value) + + +class CustomType: + """A custom datatype.""" + + pass + + +class FieldTypedExample: + """An example with field-typed descriptors.""" + + int_or_str_prop: int | str = FieldTypedBaseDescriptor() + int_or_str_subscript = FieldTypedBaseDescriptor[int | str]() + int_or_str_stringified: "int | str" = FieldTypedBaseDescriptor() + customprop: CustomType = FieldTypedBaseDescriptor() + customprop_subscript = FieldTypedBaseDescriptor[CustomType]() + futureprop: "FutureType" = FieldTypedBaseDescriptor() + + +class FutureType: + """A custom datatype, defined after the descriptor.""" + + pass + + +@pytest.mark.parametrize( + ("name", "value_type"), + [ + ("int_or_str_prop", int | str), + ("int_or_str_subscript", int | str), + ("int_or_str_stringified", int | str), + ("customprop", CustomType), + ("customprop_subscript", CustomType), + ("futureprop", FutureType), + ], +) +def test_fieldtyped_definition(name, value_type): + """Test that field-typed descriptors pick up their type correctly.""" + prop = getattr(FieldTypedExample, name) + assert prop.name == name + assert prop.value_type == value_type + + +def test_fieldtyped_missingtype(): + """Check the right error is raised when no type can be found.""" + with raises_or_is_caused_by(MissingTypeError) as excinfo: + + class Example2: + field2 = FieldTypedBaseDescriptor() + + msg = str(excinfo.value) + assert msg.startswith("No type hint was found") + # We check the field name is included, because the exception will + # arise from the end of the class definition, rather than the line + # where the field is defined. + assert "field2" in msg + + # This one defines OK, but should error when we access its type. + # Note that Ruff already spots the bad forward reference, hence the + # directive to ignore F821. + class Example3: + field3: "BadForwardReference" = FieldTypedBaseDescriptor() # noqa: F821 + field4: "int" = FieldTypedBaseDescriptor() + field5: "int" = FieldTypedBaseDescriptor() + + with pytest.raises(MissingTypeError) as excinfo: + _ = Example3.field3.value_type + + msg = str(excinfo.value) + assert "resolve forward ref" in msg + assert "field3" in msg + + # If we try to resolve a forward reference and the owner is None, it + # should raise an error. + # I don't see how this could happen in practice, _owner is always + # set if we find a forward reference. + # We force this error condition by manually setting _owner to None + Example3.field4._owner = None + + with pytest.raises(MissingTypeError) as excinfo: + _ = Example3.field4.value_type + + msg = str(excinfo.value) + assert "resolve forward ref" in msg + assert "wasn't saved" in msg + assert "field4" in msg + + # We reuse field4 but manually set _type and _unevaluated_type_hint + # to None, to test the catch-all error + Example3.field4._unevaluated_type_hint = None + Example3.field4._type = None + + with pytest.raises(MissingTypeError) as excinfo: + _ = Example3.field4.value_type + + msg = str(excinfo.value) + assert "bug in LabThings" in msg + assert "caught before now" in msg + assert "field4" in msg + + # If the class is finalised before we evaluate type hints, we should + # get a MissingTypeError. This probably only happens on dynamically + # generated classes, and I think it's unlikely we'd dynamically generate + # Thing subclasses in a way that they go out of scope. + prop = Example3.field5 + del Example3 + gc.collect() + + with pytest.raises(MissingTypeError) as excinfo: + _ = prop.value_type + + msg = str(excinfo.value) + assert "resolve forward ref" in msg + assert "garbage collected" in msg + assert "field5" in msg + + # Rather than roll my own evaluator for forward references, we just + # won't support forward references in subscripted types for now. + with raises_or_is_caused_by(MissingTypeError) as excinfo: + + class Example4: + field6 = FieldTypedBaseDescriptor["str"]() + + msg = str(excinfo.value) + assert "forward reference" in msg + assert "not supported as subscripts" + assert "field6" in msg + + +def test_mismatched_types(): + """Check two type hints that don't match raises an error.""" + with raises_or_is_caused_by(InconsistentTypeError): + + class Example3: + field: int = FieldTypedBaseDescriptor[str]() + + +def test_double_specified_types(): + """Check two type hints that match are allowed. + + This is a very odd thing to do, but it feels right to allow + it, provided the types are an exact match. + """ + + class Example4: + field: int | None = FieldTypedBaseDescriptor[int | None]() + + assert Example4.field.value_type == int | None + + +def test_stringified_vs_unstringified_mismatch(): + """Test that string type hints don't match non-string ones. + + This behaviour may change in the future - but this test is here + to make sure that, if it does, we are changing it deliberately. + If a descriptor is typed using both a subscript and a field + annotation, they should match - + """ + with raises_or_is_caused_by(InconsistentTypeError): + + class Example5: + field: "int" = FieldTypedBaseDescriptor[int]() diff --git a/tests/test_blob_output.py b/tests/test_blob_output.py index 32fdb94b..ae955320 100644 --- a/tests/test_blob_output.py +++ b/tests/test_blob_output.py @@ -71,9 +71,12 @@ def check_passthrough(self, thing_one: ThingOneDep) -> bool: @pytest.fixture def client(): """Yield a test client connected to a ThingServer.""" - server = lt.ThingServer() - server.add_thing("thing_one", ThingOne) - server.add_thing("thing_two", ThingTwo) + server = lt.ThingServer( + { + "thing_one": ThingOne, + "thing_two": ThingTwo, + } + ) with TestClient(server.app) as client: yield client diff --git a/tests/test_endpoint_decorator.py b/tests/test_endpoint_decorator.py index 6360704e..9348d43c 100644 --- a/tests/test_endpoint_decorator.py +++ b/tests/test_endpoint_decorator.py @@ -24,8 +24,7 @@ def post_method(self, body: PostBodyModel) -> str: def test_endpoints(): """Check endpoints may be added to the app and work as expected.""" - server = lt.ThingServer() - server.add_thing("thing", MyThing) + server = lt.ThingServer({"thing": MyThing}) thing = server.things["thing"] with TestClient(server.app) as client: # Check the function works when used directly diff --git a/tests/test_fallback.py b/tests/test_fallback.py index 257ccda8..bc0619a7 100644 --- a/tests/test_fallback.py +++ b/tests/test_fallback.py @@ -1,5 +1,12 @@ +"""Test the fallback server. + +If the server is started from the command line, with ``--fallback`` specified, +we start a lightweight fallback server to show an error message. This test +verifies that it works as expected. +""" + from fastapi.testclient import TestClient -from labthings_fastapi.server import server_from_config +import labthings_fastapi as lt from labthings_fastapi.server.fallback import app @@ -34,7 +41,7 @@ def test_fallback_with_error(): def test_fallback_with_server(): - config = { + config_dict = { "things": { "thing1": "labthings_fastapi.example_things:MyThing", "thing2": { @@ -43,7 +50,8 @@ def test_fallback_with_server(): }, } } - app.labthings_server = server_from_config(config) + config = lt.ThingServerConfig.model_validate(config_dict) + app.labthings_server = lt.ThingServer.from_config(config) with TestClient(app) as client: response = client.get("/") html = response.text diff --git a/tests/test_invocation_contexts.py b/tests/test_invocation_contexts.py new file mode 100644 index 00000000..36db346c --- /dev/null +++ b/tests/test_invocation_contexts.py @@ -0,0 +1,264 @@ +"""Test logging and cancellation, implemented via contextvars. + +These tests cover the code in `invocation_contexts` directly. They are also tested +in the context of a ``ThingServer`` in, for example, ``test_action_logging`` and +``test_action_cancel`` . +""" + +from contextlib import contextmanager +import time +import pytest +import uuid +from threading import Thread +from labthings_fastapi.invocation_contexts import ( + CancelEvent, + ThreadWithInvocationID, + cancellable_sleep, + fake_invocation_context, + get_cancel_event, + get_invocation_id, + invocation_id_ctx, + raise_if_cancelled, + set_invocation_id, +) +from labthings_fastapi.exceptions import ( + NoInvocationContextError, + InvocationCancelledError, +) + + +def append_invocation_id(ids: list): + """Append the current invocation ID (or the error) to a list.""" + try: + ids.append(get_invocation_id()) + except NoInvocationContextError as e: + ids.append(e) + + +def test_getting_and_setting_id(): + """Check the invocation context variable behaves as expected.""" + + # By default, the invocation id is not set + assert invocation_id_ctx.get(...) is ... + + # This means we get an error if we look for the ID + with pytest.raises(NoInvocationContextError): + get_invocation_id() + + # Once we set an ID, it should be returned + id = uuid.uuid4() + with set_invocation_id(id): + assert get_invocation_id() == id + + # It should be reset afterwards + with pytest.raises(NoInvocationContextError): + get_invocation_id() + + # A context manager lets us fake the ID for testing + with fake_invocation_context(): + assert isinstance(get_invocation_id(), uuid.UUID) + + # This should also be reset afterwards + with pytest.raises(NoInvocationContextError): + get_invocation_id() + + # A new thread will not copy the context by default, so using + # get_invocation_id in a thread will fail: + with fake_invocation_context(): + before = get_invocation_id() + ids = [] + t = Thread(target=append_invocation_id, args=[ids]) + t.start() + t.join() + after = get_invocation_id() + + assert before == after + assert len(ids) == 1 + assert isinstance(ids[0], NoInvocationContextError) + + +@contextmanager +def assert_takes_time(min_t: float | None, max_t: float | None): + """Assert that a code block takes a certain amount of time.""" + before = time.time() + yield + after = time.time() + duration = after - before + if min_t is not None: + assert duration >= min_t + if max_t is not None: + assert duration <= max_t + + +def test_cancel_event(): + """Check the cancel event works as intended.""" + id = uuid.uuid4() + event = CancelEvent.get_for_id(id) + + # We should get back the same event if we call this twice + assert event is CancelEvent.get_for_id(id) + # The function below is equivaent to the class method above. + assert event is get_cancel_event(id) + + # We should not be able to make a second one with the constructor + with pytest.raises(RuntimeError): + CancelEvent(id) + + # We make a second event with a different ID. We'll use the constructor + # directly, as this should work the first time it's called (as there is + # no existing event). + id2 = uuid.uuid4() + event2 = CancelEvent(id2) + assert event2 is CancelEvent.get_for_id(id2) + assert event2 is not event + assert get_cancel_event(id2) is event2 + + # The module-level function falls back on the context variable for ID, + # so it should raise an exception if the ID isn't present: + with pytest.raises(NoInvocationContextError): + get_cancel_event() + + # If we have an invocation ID in the context, this should succeed even + # if we've not made an event yet. + with fake_invocation_context(): + assert isinstance(get_cancel_event(), CancelEvent) + + # The two custom functions should raise `InvocationCancelledError` if + # the event is set, so we'll run them both with it set and not set. + # raise_if_set should do nothing if the event is not set. + assert not event.is_set() + event.raise_if_set() + # it should raise an exception if the event is set. + event.set() + with pytest.raises(InvocationCancelledError): + event.raise_if_set() + # When the event raises an exception, it resets - one `set()` == one error. + assert not event.is_set() + + # sleep behaves the same way, but waits a finite time. + with assert_takes_time(0.02, 0.04): + event.sleep(0.02) + # check an exception gets raised and reset if appropriate + event.set() + with pytest.raises(InvocationCancelledError): + event.sleep(1) + assert not event.is_set() + + +def test_cancellable_sleep(): + """Check the module-level cancellable sleep.""" + # with no invocation context, the function should wait + # and there should be no error. + with assert_takes_time(0.02, 0.04): + cancellable_sleep(0.02) + + with fake_invocation_context(): + event = get_cancel_event() + + # the function should wait a finite time + with assert_takes_time(0.02, 0.04): + cancellable_sleep(0.02) + + # check an exception gets raised and reset if appropriate + event.set() + with pytest.raises(InvocationCancelledError): + cancellable_sleep(1) + assert not event.is_set() + + +def test_raise_if_cancelled(): + """Check the module-level cancellable sleep.""" + # the function should return immediately. + with assert_takes_time(None, 0.002): + raise_if_cancelled() + + with fake_invocation_context(): + event = get_cancel_event() + + # the function should return immediately. + with assert_takes_time(None, 0.002): + raise_if_cancelled() + + # check an exception gets raised and reset if appropriate + event.set() + with pytest.raises(InvocationCancelledError): + raise_if_cancelled() + assert not event.is_set() + + +def test_thread_with_invocation_id(): + """Test our custom thread subclass makes a new ID and can be cancelled.""" + ids = [] + t = ThreadWithInvocationID(target=append_invocation_id, args=[ids]) + assert isinstance(t.invocation_id, uuid.UUID) + t.start() + t.join() + assert len(ids) == 1 + assert ids[0] == t.invocation_id + assert t.exception is None + assert t.result is None + + +def test_thread_with_invocation_id_cancel(): + """Test the custom thread subclass responds to cancellation.""" + # Check cancellable sleep works in the thread + t = ThreadWithInvocationID(target=cancellable_sleep, args=[1]) + assert isinstance(t.invocation_id, uuid.UUID) + t.start() + t.cancel() + with assert_takes_time(None, 0.1): + t.join() + assert isinstance(t.exception, InvocationCancelledError) + + +def test_thread_with_invocation_id_return_value(): + """Check we capture the return value when running in a ThreadWithInvocationID.""" + t = ThreadWithInvocationID(target=lambda: True) + t.start() + t.join() + assert t.exception is None + assert t.result is True + + +def run_function_in_thread_and_propagate_cancellation(func, *args): + """Run a function in a ThreadWithInvocationID.""" + t = ThreadWithInvocationID(target=func, args=args) + t.start() + try: + t.join_and_propagate_cancel(1) + except InvocationCancelledError: + # We still want to return the finished thread if it's + # cancelled. + pass + return t + + +def test_thread_with_invocation_id_cancellation_propagates(): + """Check that a cancel event can propagate to our thread. + + ``join_and_propagate_cancellation`` should cancel the spawned thread if + the parent thread is cancelled while it's waiting for the spawned thread + to join. + """ + # Check we can propagate cancellation. + # First, we run `cancellable_sleep` and check it doesn't cancel + with fake_invocation_context(): + # First test our function - there is only one thread here, and we + # check it finishes and doesn't error. + t = run_function_in_thread_and_propagate_cancellation(cancellable_sleep, 0.02) + assert isinstance(t, ThreadWithInvocationID) + assert not t.is_alive() + assert t.exception is None + + # Next, we run it in a thread, and cancel that thread. + # The error should propagate to the inner thread. + t = ThreadWithInvocationID( + target=run_function_in_thread_and_propagate_cancellation, + args=[cancellable_sleep, 10], + ) + t.start() + t.cancel() + with assert_takes_time(None, 0.05): + t.join() + assert isinstance(t.result, ThreadWithInvocationID) + assert isinstance(t.result.exception, InvocationCancelledError) diff --git a/tests/test_locking_decorator.py b/tests/test_locking_decorator.py index 798bfd1d..c2566774 100644 --- a/tests/test_locking_decorator.py +++ b/tests/test_locking_decorator.py @@ -115,8 +115,7 @@ def echo_via_client(client): def test_locking_in_server(): """Check the lock works within LabThings.""" - server = lt.ThingServer() - server.add_thing("thing", LockedExample) + server = lt.ThingServer({"thing": LockedExample}) thing = server.things["thing"] with TestClient(server.app) as client: # Start a long task diff --git a/tests/test_logs.py b/tests/test_logs.py new file mode 100644 index 00000000..a366776c --- /dev/null +++ b/tests/test_logs.py @@ -0,0 +1,178 @@ +"""Unit tests for the `.logs` module. + +These tests are intended to complement the more functional tests +in ``test_action_logging`` with bottom-up tests for code in the +`.logs` module. +""" + +from collections import deque +import logging +from types import EllipsisType +import pytest +from uuid import UUID, uuid4 +from labthings_fastapi import logs +from labthings_fastapi.invocation_contexts import ( + fake_invocation_context, + set_invocation_id, +) +import labthings_fastapi as lt +from labthings_fastapi.exceptions import LogConfigurationError +from labthings_fastapi.thing_server_interface import create_thing_without_server + + +class ThingThatLogs(lt.Thing): + @lt.thing_action + def log_a_message(self, msg: str): + """Log a message to the thing's logger.""" + self.logger.info(msg) + + +def reset_thing_logger(): + """Remove all handlers from the THING_LOGGER to reset it.""" + logger = logs.THING_LOGGER + # Note that the [:] below is important: this copies the list and avoids + # issues with modifying a list as we're iterating through it. + for h in logger.handlers[:]: + logger.removeHandler(h) + for f in logger.filters[:]: + logger.removeFilter(f) + assert len(logger.handlers) == 0 + assert len(logger.filters) == 0 + + +def make_record(msg="A test message", id: UUID | EllipsisType | None = ...): + """A LogRecord object.""" + record = logging.LogRecord( + "labthings_fastapi.things.test", + logging.INFO, + "test/file/path.py", + 42, + msg, + None, + None, + "test_function", + None, + ) + if id is not ...: + record.invocation_id = id + return record + + +def test_inject_invocation_id_nocontext(): + """Check our filter function correctly adds invocation ID to a log record.""" + record = make_record() + # The record won't have an invocation ID to start with. + assert not hasattr(record, "invocation_id") + # The filter should return True (to keep the record) + assert logs.inject_invocation_id(record) is True + # It should add the attribute, but with no invocation + # context, it should be set to None + assert record.invocation_id is None + + # Currently, if we re-run the filter it silently overwrites, + # so there should be no error below: + assert logs.inject_invocation_id(record) is True + + # Currently, it should overwrite the value. This behaviour + # possibly wants to change in the future, and this test + # should be updated. + with fake_invocation_context() as id: + assert logs.inject_invocation_id(record) is True + assert record.invocation_id == id + + +def test_inject_invocation_id_withcontext(): + """Check our filter function correctly adds invocation ID to a log record.""" + record = make_record() + # The record won't have an invocation ID to start with. + assert not hasattr(record, "invocation_id") + # The filter should return True (to keep the record) + with fake_invocation_context() as id: + assert logs.inject_invocation_id(record) is True + assert record.invocation_id == id + + # Currently, it should overwrite the value. This behaviour + # possibly wants to change in the future, and this test + # should be updated. This ID should be a fresh one. + with fake_invocation_context() as id2: + assert logs.inject_invocation_id(record) is True + # Check the ID has changed and was overwritten. + assert id2 != id + assert record.invocation_id == id2 + + +def test_dequebyinvocationidhandler(): + """Check the custom log handler works as expected.""" + handler = logs.DequeByInvocationIDHandler() + assert handler.level == logging.INFO + + destinations = { + uuid4(): deque(), + uuid4(): deque(), + } + # We should be able to log with nothing set up, the record + # won't go anywhere but there shouldn't be any errors. + for id in destinations.keys(): + handler.emit(make_record(id=id)) + for dest in destinations.values(): + assert len(dest) == 0 + + # After adding the destinations, the logs should appear. + for id, dest in destinations.items(): + handler.add_destination_for_id(id, dest) + + for id in destinations.keys(): + handler.emit(make_record(id=id)) + for id, dest in destinations.items(): + assert len(dest) == 1 + assert dest[0].invocation_id == id + + +def test_configure_thing_logger(): + """Check the logger is configured correctly.""" + # Start by resetting the handlers on the logger + reset_thing_logger() + + # Then configure it + logs.configure_thing_logger() + + # Check it's correct + assert logs.THING_LOGGER.level == logging.INFO + assert len(logs.THING_LOGGER.handlers) == 1 + assert isinstance(logs.THING_LOGGER.handlers[0], logs.DequeByInvocationIDHandler) + + # Test it out + with fake_invocation_context() as id: + dest = deque() + logs.add_thing_log_destination(id, dest) + logger = logs.THING_LOGGER.getChild("foo") + logger.info("Test") + assert len(dest) == 1 + assert dest[0].msg == "Test" + + +def test_add_thing_log_destination(): + """Check the module-level function to add an invocation log destination.""" + reset_thing_logger() + id = uuid4() + dest = deque() + + with pytest.raises(LogConfigurationError): + # This won't work until the handler is added + logs.add_thing_log_destination(id, dest) + + logs.THING_LOGGER.addHandler(logs.DequeByInvocationIDHandler()) + logs.THING_LOGGER.addHandler(logs.DequeByInvocationIDHandler()) + with pytest.raises(LogConfigurationError): + # More than one handler will also make it fail with an error. + logs.add_thing_log_destination(id, dest) + + reset_thing_logger() + logs.configure_thing_logger() + + thing = create_thing_without_server(ThingThatLogs) + logs.add_thing_log_destination(id, dest) + with set_invocation_id(id): + thing.log_a_message("Test Message.") + assert len(dest) == 1 + assert dest[0].getMessage() == "Test Message." diff --git a/tests/test_mjpeg_stream.py b/tests/test_mjpeg_stream.py index a780d10c..0effde36 100644 --- a/tests/test_mjpeg_stream.py +++ b/tests/test_mjpeg_stream.py @@ -46,8 +46,7 @@ def _make_images(self): @pytest.fixture def client(): """Yield a test client connected to a ThingServer""" - server = lt.ThingServer() - server.add_thing("telly", Telly) + server = lt.ThingServer({"telly": Telly}) with TestClient(server.app) as client: yield client @@ -74,8 +73,9 @@ def test_mjpeg_stream(client): if __name__ == "__main__": import uvicorn - server = lt.ThingServer() - telly = server.add_thing("telly", Telly) + server = lt.ThingServer({"telly": Telly}) + telly = server.things["telly"] + assert isinstance(telly, Telly) telly.framerate = 6 telly.frame_limit = -1 uvicorn.run(server.app, port=5000) diff --git a/tests/test_properties.py b/tests/test_properties.py index a50a8b4a..1bbbbd9c 100644 --- a/tests/test_properties.py +++ b/tests/test_properties.py @@ -6,6 +6,7 @@ import pytest import labthings_fastapi as lt +from labthings_fastapi.exceptions import ServerNotRunningError from .temp_client import poll_task @@ -51,8 +52,7 @@ def toggle_boolprop_from_thread(self): @pytest.fixture def server(): - server = lt.ThingServer() - server.add_thing("thing", PropertyTestThing) + server = lt.ThingServer({"thing": PropertyTestThing}) return server @@ -230,3 +230,15 @@ def test_setting_from_thread(server): r = client.get("/thing/boolprop") assert r.status_code == 200 assert r.json() is True + + +def test_setting_without_event_loop(): + """Test DataProperty raises an error if set without an event loop.""" + # This test may need to change, if we change the intended behaviour + # Currently it should never be necessary to change properties from the + # main thread, so we raise an error if you try to do so + server = lt.ThingServer({"thing": PropertyTestThing}) + thing = server.things["thing"] + assert isinstance(thing, PropertyTestThing) + with pytest.raises(ServerNotRunningError): + thing.boolprop = False # Can't call it until the event loop's running diff --git a/tests/test_property.py b/tests/test_property.py index 96ceef68..24321edb 100644 --- a/tests/test_property.py +++ b/tests/test_property.py @@ -15,9 +15,18 @@ from fastapi.testclient import TestClient import pydantic import pytest -from labthings_fastapi import properties as tp +from labthings_fastapi import properties +from labthings_fastapi.properties import ( + BaseProperty, + DataProperty, + FunctionalProperty, + MissingDefaultError, + OverspecifiedDefaultError, + default_factory_from_arguments, +) from labthings_fastapi.base_descriptor import DescriptorAddedToClassTwiceError -from labthings_fastapi.exceptions import NotConnectedToServerError +from labthings_fastapi.exceptions import MissingTypeError, NotConnectedToServerError +import labthings_fastapi as lt from .utilities import raises_or_is_caused_by @@ -29,26 +38,26 @@ def test_default_factory_from_arguments(): are provided, or if none are provided. """ # Check for an error with no arguments - with pytest.raises(tp.MissingDefaultError): - tp.default_factory_from_arguments() + with pytest.raises(MissingDefaultError): + default_factory_from_arguments() # Check for an error with both arguments - with pytest.raises(tp.OverspecifiedDefaultError): - tp.default_factory_from_arguments([], list) + with pytest.raises(OverspecifiedDefaultError): + default_factory_from_arguments([], list) # Check a factory is passed unchanged - assert tp.default_factory_from_arguments(..., list) is list + assert default_factory_from_arguments(..., list) is list # Check a value is wrapped in a factory - factory = tp.default_factory_from_arguments(True, None) + factory = default_factory_from_arguments(True, None) assert factory() is True # Check there's an error if our default factory isn't callable - with pytest.raises(tp.MissingDefaultError): - tp.default_factory_from_arguments(default_factory=False) + with pytest.raises(MissingDefaultError): + default_factory_from_arguments(default_factory=False) # Check None works as a default value - factory = tp.default_factory_from_arguments(default=None) + factory = default_factory_from_arguments(default=None) assert factory() is None @@ -79,11 +88,11 @@ def mock_and_capture_args(monkeypatch, target, name): monkeypatch.setattr(target, name, MockClass) -@pytest.mark.parametrize("func", [tp.property, tp.setting]) +@pytest.mark.parametrize("func", [lt.property, lt.setting]) def test_toplevel_function(monkeypatch, func): """Test the various ways in which `lt.property` or `lt.setting` may be invoked. - This test is parametrized, so `func` will be either `tp.property` or `tp.setting`. + This test is parametrized, so `func` will be either `lt.property` or `lt.setting`. We then look up the corresponding descriptor classes. It's unfortunate that the body of this test is a bit generic, but as both @@ -95,10 +104,10 @@ def test_toplevel_function(monkeypatch, func): # Mock DataProperty,FunctionalProperty or the equivalent for settings # suffix will be "Property" or "Setting" suffix = func.__name__.capitalize() - mock_and_capture_args(monkeypatch, tp, f"Data{suffix}") - mock_and_capture_args(monkeypatch, tp, f"Functional{suffix}") - DataClass = getattr(tp, f"Data{suffix}") - FunctionalClass = getattr(tp, f"Functional{suffix}") + mock_and_capture_args(monkeypatch, properties, f"Data{suffix}") + mock_and_capture_args(monkeypatch, properties, f"Functional{suffix}") + DataClass = getattr(properties, f"Data{suffix}") + FunctionalClass = getattr(properties, f"Functional{suffix}") def getter(self) -> str: return "test" @@ -132,19 +141,19 @@ def getter(self) -> str: # The positional argument is the setter, so `None` is not valid # and probably means someone forgot to add `default=`. - with pytest.raises(tp.MissingDefaultError): + with pytest.raises(MissingDefaultError): func(None) # Calling with no arguments is also not valid and raises an error - with pytest.raises(tp.MissingDefaultError): + with pytest.raises(MissingDefaultError): func() # If more than one default is specified, we should raise an error. - with pytest.raises(tp.OverspecifiedDefaultError): + with pytest.raises(OverspecifiedDefaultError): func(default=[], default_factory=list) - with pytest.raises(tp.OverspecifiedDefaultError): + with pytest.raises(OverspecifiedDefaultError): func(getter, default=[]) - with pytest.raises(tp.OverspecifiedDefaultError): + with pytest.raises(OverspecifiedDefaultError): func(getter, default_factory=list) @@ -154,19 +163,20 @@ def test_baseproperty_type_and_model(): This checks baseproperty correctly wraps plain types in a `pydantic.RootModel`. """ - prop = tp.BaseProperty() - # By default, we have no type so `.type` errors. - with pytest.raises(tp.MissingTypeError): - _ = prop.value_type - with pytest.raises(tp.MissingTypeError): - _ = prop.model + with raises_or_is_caused_by(MissingTypeError): - # Once _type is set, these should both work. - prop._type = str | None - assert str(prop.value_type) == "str | None" - assert issubclass(prop.model, pydantic.RootModel) - assert str(prop.model.model_fields["root"].annotation) == "str | None" + class Example: + prop = BaseProperty() + + class Example: + prop: "str | None" = BaseProperty() + + assert isinstance(None, Example.prop.value_type) + assert isinstance("test", Example.prop.value_type) + assert str(Example.prop.value_type) == "str | None" + assert issubclass(Example.prop.model, pydantic.RootModel) + assert str(Example.prop.model.model_fields["root"].annotation) == "str | None" def test_baseproperty_type_and_model_pydantic(): @@ -175,16 +185,16 @@ def test_baseproperty_type_and_model_pydantic(): This checks baseproperty behaves correctly when its type is a BaseModel instance. """ - prop = tp.BaseProperty() class MyModel(pydantic.BaseModel): foo: str bar: int - # Once _type is set, these should both work. - prop._type = MyModel - assert prop.value_type is MyModel - assert prop.model is MyModel + class Example: + prop: MyModel = BaseProperty() + + assert Example.prop.value_type is MyModel + assert Example.prop.model is MyModel def test_baseproperty_add_to_fastapi(): @@ -192,7 +202,7 @@ def test_baseproperty_add_to_fastapi(): # Subclass to add __set__ (which is missing on BaseProperty as it's # implemented by subclasses). - class MyProperty(tp.BaseProperty): + class MyProperty(BaseProperty): def __set__(self, obj, val): pass @@ -223,12 +233,12 @@ class Example: def test_baseproperty_set_error(): """Check `.Baseproperty.__set__` raises an error and is overridden.""" - assert tp.DataProperty.__get__ is tp.BaseProperty.__get__ - assert tp.DataProperty.__set__ is not tp.BaseProperty.__set__ - assert tp.FunctionalProperty.__set__ is not tp.BaseProperty.__set__ + assert DataProperty.__get__ is BaseProperty.__get__ + assert DataProperty.__set__ is not BaseProperty.__set__ + assert FunctionalProperty.__set__ is not BaseProperty.__set__ class Example: - bp: int = tp.BaseProperty() + bp: int = BaseProperty() example = Example() with pytest.raises(NotImplementedError): @@ -249,12 +259,12 @@ def test_decorator_exception(): class BadExample: """A class with a wrongly reused descriptor.""" - prop1: int = tp.property(default=0) + prop1: int = lt.property(default=0) prop2: int = prop1 # The example below should be exempted from the rule, i.e. no error class Example: - @tp.property + @lt.property def prop(self) -> bool: """An example getter.""" @@ -263,9 +273,9 @@ def set_prop(self, val: bool) -> None: """A setter named differently.""" pass - assert isinstance(Example.prop, tp.FunctionalProperty) + assert isinstance(Example.prop, FunctionalProperty) assert Example.prop.name == "prop" - assert not isinstance(Example.set_prop, tp.FunctionalProperty) + assert not isinstance(Example.set_prop, FunctionalProperty) assert callable(Example.set_prop) @@ -275,7 +285,7 @@ def test_premature_api_and_affordance(mocker): class Example: path = None # this is supplied by `lt.Thing` but we're not subclassing. - @tp.property + @lt.property def prop(self) -> bool: """An example getter.""" return True diff --git a/tests/test_server.py b/tests/test_server.py index bf12abb3..fcf85a30 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -7,10 +7,14 @@ """ import pytest -from labthings_fastapi import server as ts +import labthings_fastapi as lt def test_server_from_config_non_thing_error(): """Test a typeerror is raised if something that's not a Thing is added.""" with pytest.raises(TypeError, match="not a Thing"): - ts.server_from_config({"things": {"thingone": {"class": "builtins:object"}}}) + lt.ThingServer.from_config( + lt.ThingServerConfig( + things={"thingone": lt.ThingConfig(cls="builtins:object")} + ) + ) diff --git a/tests/test_server_cli.py b/tests/test_server_cli.py index 0ec3a84b..2a391091 100644 --- a/tests/test_server_cli.py +++ b/tests/test_server_cli.py @@ -7,7 +7,6 @@ import pytest from labthings_fastapi import ThingServer -from labthings_fastapi.server import server_from_config from labthings_fastapi.server.cli import serve_from_cli @@ -82,7 +81,7 @@ def run_monitored(self, terminate_outputs=None, timeout=10): def test_server_from_config(): """Check we can create a server from a config object""" - server = server_from_config(CONFIG) + server = ThingServer.from_config(CONFIG) assert isinstance(server, ThingServer) @@ -138,8 +137,9 @@ def test_invalid_thing(): } } ) - with raises(ImportError): + with raises(SystemExit) as excinfo: check_serve_from_cli(["-j", config_json]) + assert excinfo.value.code == 3 @pytest.mark.slow diff --git a/tests/test_server_config_model.py b/tests/test_server_config_model.py new file mode 100644 index 00000000..aefb7164 --- /dev/null +++ b/tests/test_server_config_model.py @@ -0,0 +1,98 @@ +r"""Test code for `.server.config_model`\ .""" + +from pydantic import ValidationError +import pytest +from labthings_fastapi.server import config_model as cm +import labthings_fastapi.example_things +from labthings_fastapi.example_things import MyThing + + +def test_ThingConfig(): + """Test the ThingConfig model loads classes as expected.""" + # We should be able to create a valid config with just a class + direct = cm.ThingConfig(cls=labthings_fastapi.example_things.MyThing) + # Equivalently, we should be able to pass a string + fromstr = cm.ThingConfig(cls="labthings_fastapi.example_things:MyThing") + assert direct.cls is MyThing + assert fromstr.cls is MyThing + # In the absence of supplied arguments, default factories should be used + assert len(direct.args) == 0 + assert direct.kwargs == {} + assert direct.thing_slots == {} + + with pytest.raises(ValidationError, match="No module named"): + cm.ThingConfig(cls="missing.module") + + +VALID_THING_CONFIGS = { + "direct": MyThing, + "string": "labthings_fastapi.example_things:MyThing", + "model_d": cm.ThingConfig(cls=MyThing), + "model_s": cm.ThingConfig(cls="labthings_fastapi.example_things:MyThing"), + "dict_d": {"cls": MyThing}, + "dict_da": {"class": MyThing}, + "dict_s": {"cls": "labthings_fastapi.example_things:MyThing"}, + "dict_sa": {"class": "labthings_fastapi.example_things:MyThing"}, +} + + +INVALID_THING_CONFIGS = [ + {}, + {"foo": "bar"}, + {"class": MyThing, "kwargs": 1}, + "missing.module:object", + 4, + None, + False, +] + + +VALID_THING_NAMES = [ + "my_thing", + "MyThing", + "Something", + "f90785342", + "1", +] + +INVALID_THING_NAMES = [ + "", + "spaces in name", + "special * chars", + False, + 1, + "/", + "thing/with/slashes", + "trailingslash/", + "/leadingslash", +] + + +def test_ThingServerConfig(): + """Check validation of the whole server config.""" + # Things should be able to be specified as a string, a class, or a ThingConfig + config = cm.ThingServerConfig(things=VALID_THING_CONFIGS) + assert len(config.thing_configs) == 8 + for v in config.thing_configs.values(): + assert v.cls is MyThing + + # When we validate from a dict, the same options work + config = cm.ThingServerConfig.model_validate({"things": VALID_THING_CONFIGS}) + assert len(config.thing_configs) == 8 + for v in config.thing_configs.values(): + assert v.cls is MyThing + + # Check invalid configs are picked up + for spec in INVALID_THING_CONFIGS: + with pytest.raises(ValidationError): + cm.ThingServerConfig(things={"thing": spec}) + + # Check valid names are allowed + for name in VALID_THING_NAMES: + sc = cm.ThingServerConfig(things={name: MyThing}) + assert sc.thing_configs[name].cls is MyThing + + # Check bad names raise errors + for name in INVALID_THING_NAMES: + with pytest.raises(ValidationError): + cm.ThingServerConfig(things={name: MyThing}) diff --git a/tests/test_settings.py b/tests/test_settings.py index b138dcd0..aea1520e 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -146,8 +146,9 @@ def directly_set_localonly_boolsetting( test_thing.localonly_boolsetting = val -def _get_setting_file(server, thingpath): - path = os.path.join(server.settings_folder, thingpath.lstrip("/"), "settings.json") +def _get_setting_file(server: lt.ThingServer, name: str): + """Find the location of the settings file for a given Thing on a server.""" + path = server.things[name]._thing_server_interface.settings_file_path return os.path.normpath(path) @@ -176,11 +177,12 @@ def _settings_dict( @pytest.fixture -def server(): +def tempdir(): + """A temporary directory""" with tempfile.TemporaryDirectory() as tempdir: - # Yield server rather than return so that the temp directory isn't cleaned up + # Yield rather than return so that the temp directory isn't cleaned up # until after the test is run - yield lt.ThingServer(settings_folder=tempdir) + yield tempdir def test_setting_available(): @@ -193,13 +195,13 @@ def test_setting_available(): assert thing.dictsetting == {"a": 1, "b": 2} -def test_functional_settings_save(server): +def test_functional_settings_save(tempdir): """Check updated settings are saved to disk ``floatsetting`` is a functional setting, we should also test a `.DataSetting` for completeness.""" - setting_file = _get_setting_file(server, "/thing") - server.add_thing("thing", ThingWithSettings) + server = lt.ThingServer({"thing": ThingWithSettings}, settings_folder=tempdir) + setting_file = _get_setting_file(server, "thing") # No setting file created when first added assert not os.path.isfile(setting_file) with TestClient(server.app) as client: @@ -219,13 +221,13 @@ def test_functional_settings_save(server): assert json.load(file_obj) == _settings_dict(floatsetting=2.0) -def test_data_settings_save(server): +def test_data_settings_save(tempdir): """Check updated settings are saved to disk This uses ``intsetting`` which is a `.DataSetting` so it tests a different code path to the functional setting above.""" - setting_file = _get_setting_file(server, "/thing") - server.add_thing("thing", ThingWithSettings) + server = lt.ThingServer({"thing": ThingWithSettings}, settings_folder=tempdir) + setting_file = _get_setting_file(server, "thing") # The settings file should not be created yet - it's created the # first time we write to a setting. assert not os.path.isfile(setting_file) @@ -256,7 +258,7 @@ def test_data_settings_save(server): "method", ["http", "direct_thing_client", "direct"], ) -def test_readonly_setting(server, endpoint, value, method): +def test_readonly_setting(tempdir, endpoint, value, method): """Check read-only functional settings cannot be set remotely. Functional settings must always have a setter, and will be @@ -271,9 +273,11 @@ def test_readonly_setting(server, endpoint, value, method): The test is parametrized so it will run 6 times, trying one block of code inside the ``with`` block each time. """ - setting_file = _get_setting_file(server, "/thing") - server.add_thing("thing", ThingWithSettings) - server.add_thing("client_thing", ClientThing) + server = lt.ThingServer( + things={"thing": ThingWithSettings, "client_thing": ClientThing}, + settings_folder=tempdir, + ) + setting_file = _get_setting_file(server, "thing") # No setting file created when first added assert not os.path.isfile(setting_file) @@ -319,10 +323,12 @@ def test_readonly_setting(server, endpoint, value, method): assert not os.path.isfile(setting_file) # No file created -def test_settings_dict_save(server): +def test_settings_dict_save(tempdir): """Check settings are saved if the dict is updated in full""" - setting_file = _get_setting_file(server, "/thing") - thing = server.add_thing("thing", ThingWithSettings) + server = lt.ThingServer({"thing": ThingWithSettings}, settings_folder=tempdir) + setting_file = _get_setting_file(server, "thing") + thing = server.things["thing"] + assert isinstance(thing, ThingWithSettings) # No setting file created when first added assert not os.path.isfile(setting_file) with TestClient(server.app): @@ -333,14 +339,16 @@ def test_settings_dict_save(server): assert json.load(file_obj) == _settings_dict(dictsetting={"c": 3}) -def test_settings_dict_internal_update(server): +def test_settings_dict_internal_update(tempdir): """Confirm settings are not saved if the internal value of a dictionary is updated This behaviour is not ideal, but it is documented. If the behaviour is updated then the documentation should be updated and this test removed """ - setting_file = _get_setting_file(server, "/thing") - thing = server.add_thing("thing", ThingWithSettings) + server = lt.ThingServer({"thing": ThingWithSettings}, settings_folder=tempdir) + setting_file = _get_setting_file(server, "thing") + thing = server.things["thing"] + assert isinstance(thing, ThingWithSettings) # No setting file created when first added assert not os.path.isfile(setting_file) with TestClient(server.app): @@ -350,64 +358,81 @@ def test_settings_dict_internal_update(server): assert not os.path.isfile(setting_file) -def test_settings_load(server): +def test_settings_load(tempdir): """Check settings can be loaded from disk when added to server""" - setting_file = _get_setting_file(server, "/thing") + server = lt.ThingServer({"thing": ThingWithSettings}, settings_folder=tempdir) + setting_file = _get_setting_file(server, "thing") + del server setting_json = json.dumps(_settings_dict(floatsetting=3.0, stringsetting="bar")) # Create setting file - os.makedirs(os.path.dirname(setting_file)) with open(setting_file, "w", encoding="utf-8") as file_obj: file_obj.write(setting_json) # Add thing to server and check new settings are loaded - thing = server.add_thing("thing", ThingWithSettings) + server = lt.ThingServer({"thing": ThingWithSettings}, settings_folder=tempdir) + thing = server.things["thing"] + assert isinstance(thing, ThingWithSettings) assert not thing.boolsetting assert thing.stringsetting == "bar" assert thing.floatsetting == 3.0 -def test_load_extra_settings(server, caplog): +def test_load_extra_settings(caplog, tempdir): """Load from setting file. Extra setting in file should create a warning.""" - setting_file = _get_setting_file(server, "/thing") + server = lt.ThingServer({"thing": ThingWithSettings}, settings_folder=tempdir) + setting_file = _get_setting_file(server, "thing") + del server setting_dict = _settings_dict(floatsetting=3.0, stringsetting="bar") setting_dict["extra_setting"] = 33.33 setting_json = json.dumps(setting_dict) # Create setting file - os.makedirs(os.path.dirname(setting_file)) with open(setting_file, "w", encoding="utf-8") as file_obj: file_obj.write(setting_json) with caplog.at_level(logging.WARNING): - # Add thing to server - thing = server.add_thing("thing", ThingWithSettings) + # Create the server with the Thing added. + server = lt.ThingServer({"thing": ThingWithSettings}, settings_folder=tempdir) assert len(caplog.records) == 1 assert caplog.records[0].levelname == "WARNING" assert caplog.records[0].name == "labthings_fastapi.thing" + # Get the instance of the ThingWithSettings + thing = server.things["thing"] + assert isinstance(thing, ThingWithSettings) + # Check other settings are loaded as expected assert not thing.boolsetting assert thing.stringsetting == "bar" assert thing.floatsetting == 3.0 -def test_try_loading_corrupt_settings(server, caplog): +def test_try_loading_corrupt_settings(tempdir, caplog): """Load from setting file. Extra setting in file should create a warning.""" - setting_file = _get_setting_file(server, "/thing") + # Create the server once, so we can get the settings path + server = lt.ThingServer({"thing": ThingWithSettings}, settings_folder=tempdir) + setting_file = _get_setting_file(server, "thing") + del server + + # Construct a broken settings file setting_dict = _settings_dict(floatsetting=3.0, stringsetting="bar") setting_json = json.dumps(setting_dict) # Cut the start off the json to so it can't be decoded. setting_json = setting_json[3:] # Create setting file - os.makedirs(os.path.dirname(setting_file)) with open(setting_file, "w", encoding="utf-8") as file_obj: file_obj.write(setting_json) + # Recreate the server and check for warnings with caplog.at_level(logging.WARNING): # Add thing to server - thing = server.add_thing("thing", ThingWithSettings) + server = lt.ThingServer({"thing": ThingWithSettings}, settings_folder=tempdir) assert len(caplog.records) == 1 assert caplog.records[0].levelname == "WARNING" assert caplog.records[0].name == "labthings_fastapi.thing" + # Get the instance of the ThingWithSettings + thing = server.things["thing"] + assert isinstance(thing, ThingWithSettings) + # Check default settings are loaded assert not thing.boolsetting assert thing.stringsetting == "foo" diff --git a/tests/test_thing.py b/tests/test_thing.py index 375c33e5..988a0648 100644 --- a/tests/test_thing.py +++ b/tests/test_thing.py @@ -11,6 +11,5 @@ def test_td_validates(): def test_add_thing(): """Check that thing can be added to the server""" - server = ThingServer() - server.add_thing("thing", MyThing) + server = ThingServer({"thing": MyThing}) assert isinstance(server.things["thing"], MyThing) diff --git a/tests/test_thing_lifecycle.py b/tests/test_thing_lifecycle.py index 6be75547..e737506d 100644 --- a/tests/test_thing_lifecycle.py +++ b/tests/test_thing_lifecycle.py @@ -1,8 +1,9 @@ +import pytest import labthings_fastapi as lt from fastapi.testclient import TestClient -class TestThing(lt.Thing): +class LifecycleThing(lt.Thing): alive: bool = lt.property(default=False) "Whether the thing is alive." @@ -16,11 +17,19 @@ def __exit__(self, *args): self.alive = False -server = lt.ThingServer() -thing = server.add_thing("thing", TestThing) +@pytest.fixture +def server(): + """A ThingServer with a LifecycleThing.""" + return lt.ThingServer({"thing": LifecycleThing}) -def test_thing_alive(): +@pytest.fixture +def thing(server): + """The thing attached to our server.""" + return server.things["thing"] + + +def test_thing_alive(server, thing): assert thing.alive is False with TestClient(server.app) as client: assert thing.alive is True @@ -29,7 +38,7 @@ def test_thing_alive(): assert thing.alive is False -def test_thing_alive_twice(): +def test_thing_alive_twice(server, thing): """It's unlikely we need to stop and restart the server within one Python session, except for testing. This test should explicitly make sure our lifecycle stuff is closing down cleanly and can restart. diff --git a/tests/test_thing_server_interface.py b/tests/test_thing_server_interface.py index 82914d01..899ed48a 100644 --- a/tests/test_thing_server_interface.py +++ b/tests/test_thing_server_interface.py @@ -9,7 +9,12 @@ import labthings_fastapi as lt from labthings_fastapi.exceptions import ServerNotRunningError -from labthings_fastapi import thing_server_interface as tsi +from labthings_fastapi.thing_server_interface import ( + MockThingServerInterface, + ThingServerInterface, + ThingServerMissingError, + create_thing_without_server, +) NAME = "testname" @@ -26,21 +31,23 @@ def thing_state(self): def server(): """Return a LabThings server""" with tempfile.TemporaryDirectory() as dir: - server = lt.ThingServer(settings_folder=dir) - server.add_thing("example", ExampleThing) + server = lt.ThingServer( + things={"example": ExampleThing}, + settings_folder=dir, + ) yield server @pytest.fixture def interface(server): """Return a ThingServerInterface, connected to a server.""" - return tsi.ThingServerInterface(server, NAME) + return ThingServerInterface(server, NAME) @pytest.fixture def mockinterface(): """Return a MockThingServerInterface.""" - return tsi.MockThingServerInterface(NAME) + return MockThingServerInterface(NAME) def test_get_server(server, interface): @@ -57,12 +64,12 @@ def test_get_server_error(): This is an error condition that I would find surprising if it ever occurred, but it's worth checking. """ - server = lt.ThingServer() - interface = tsi.ThingServerInterface(server, NAME) + server = lt.ThingServer(things={}) + interface = ThingServerInterface(server, NAME) assert interface._get_server() is server del server gc.collect() - with pytest.raises(tsi.ThingServerMissingError): + with pytest.raises(ThingServerMissingError): interface._get_server() @@ -162,7 +169,18 @@ def test_mock_get_thing_states(mockinterface): def test_create_thing_without_server(): """Check the test harness for creating things without a server.""" - example = tsi.create_thing_without_server(ExampleThing) + example = create_thing_without_server(ExampleThing) assert isinstance(example, ExampleThing) assert example.path == "/examplething/" - assert isinstance(example._thing_server_interface, tsi.MockThingServerInterface) + assert isinstance(example._thing_server_interface, MockThingServerInterface) + + # Check we can specify the settings location + with tempfile.TemporaryDirectory() as folder: + ex2 = create_thing_without_server(ExampleThing, settings_folder=folder) + assert ex2._thing_server_interface.settings_file_path == os.path.join( + folder, "settings.json" + ) + + # We can't supply the interface as a kwarg + with pytest.raises(ValueError, match="may not supply"): + create_thing_without_server(ExampleThing, thing_server_interface=None) diff --git a/tests/test_thing_slots.py b/tests/test_thing_slots.py new file mode 100644 index 00000000..08f83161 --- /dev/null +++ b/tests/test_thing_slots.py @@ -0,0 +1,451 @@ +"""Test the thing_slot module.""" + +from collections.abc import Mapping +import gc +import pytest +import labthings_fastapi as lt +from fastapi.testclient import TestClient + +from labthings_fastapi.exceptions import ThingSlotError + + +class ThingOne(lt.Thing): + """A class that will cause chaos if it can.""" + + other_thing: "ThingTwo" = lt.thing_slot() + n_things: "Mapping[str, ThingThree]" = lt.thing_slot() + optional_thing: "ThingThree | None" = lt.thing_slot() + + +class ThingTwo(lt.Thing): + """A class that relies on ThingOne.""" + + other_thing: ThingOne = lt.thing_slot() + + +class ThingN(lt.Thing): + """A class that emulates ThingOne and ThingTwo more generically.""" + + other_thing: "ThingN" = lt.thing_slot(None) + + +class ThingThree(lt.Thing): + """A Thing that has no other attributes.""" + + pass + + +class ThingThatMustBeConfigured(lt.Thing): + """A Thing that has a default that won't work.""" + + other_thing: lt.Thing = lt.thing_slot(None) + + +class Dummy: + """A dummy thing-like class.""" + + def __init__(self, name): + """Set the dummy Thing's name.""" + self.name = name + + +class Dummy1(Dummy): + """A subclass of Dummy.""" + + +class Dummy2(Dummy): + """A different subclass of Dummy.""" + + +class ThingWithManyConnections: + """A class with lots of ThingSlots. + + This class is not actually meant to be used - it is a host for + the thing_slot attributes. It's not a Thing, to simplify + testing. The "thing" types it depends on are also not Things, + again to simplify testing. + """ + + name = "thing" + + single_no_default: Dummy1 = lt.thing_slot() + optional_no_default: Dummy1 | None = lt.thing_slot() + multiple_no_default: Mapping[str, Dummy1] = lt.thing_slot() + + single_default_none: Dummy1 = lt.thing_slot(None) + optional_default_none: Dummy1 | None = lt.thing_slot(None) + multiple_default_none: Mapping[str, Dummy1] = lt.thing_slot(None) + + single_default_str: Dummy1 = lt.thing_slot("dummy_a") + optional_default_str: Dummy1 | None = lt.thing_slot("dummy_a") + multiple_default_str: Mapping[str, Dummy1] = lt.thing_slot("dummy_a") + + single_default_seq: Dummy1 = lt.thing_slot(["dummy_a", "dummy_b"]) + optional_default_seq: Dummy1 | None = lt.thing_slot(["dummy_a", "dummy_b"]) + multiple_default_seq: Mapping[str, Dummy1] = lt.thing_slot(["dummy_a", "dummy_b"]) + + +class ThingWithFutureConnection: + """A class with a ThingSlot in the future.""" + + name = "thing" + + single: "DummyFromTheFuture" = lt.thing_slot() + optional: "DummyFromTheFuture | None" = lt.thing_slot() + multiple: "Mapping[str, DummyFromTheFuture]" = lt.thing_slot() + + +class DummyFromTheFuture(Dummy): + """A subclass of the dummy Thing defined after the dependent class.""" + + +def dummy_things(names, cls=Dummy1): + """Turn a list or set of names into a dict of Things.""" + return {n: cls(n) for n in names} + + +@pytest.fixture +def mixed_things(): + """A list of Things with two different types.""" + return { + **dummy_things({"thing1_a", "thing1_b"}, Dummy1), + **dummy_things({"thing2_a", "thing2_b"}, Dummy2), + } + + +CONN_TYPES = ["single", "optional", "multiple"] +DEFAULTS = ["no_default", "default_none", "default_str", "default_seq"] + + +@pytest.mark.parametrize("conn_type", CONN_TYPES) +@pytest.mark.parametrize("default", DEFAULTS) +def test_type_analysis(conn_type, default): + """Check the type of things and thing connections is correctly determined.""" + attr = getattr(ThingWithManyConnections, f"{conn_type}_{default}") + + # All the attributes use the same type of Thing, Dummy1 + assert attr.thing_type == (Dummy1,) + assert attr.is_optional is (conn_type == "optional") + assert attr.is_mapping is (conn_type == "multiple") + + +@pytest.mark.parametrize("conn_type", CONN_TYPES) +def test_type_analysis_strings(conn_type): + """Check connection types still work with stringified annotations.""" + attr = getattr(ThingWithFutureConnection, f"{conn_type}") + + # All the attributes use the same type of Thing, Dummy1 + assert attr.thing_type == (DummyFromTheFuture,) + assert attr.is_optional is (conn_type == "optional") + assert attr.is_mapping is (conn_type == "multiple") + + +def test_pick_things(mixed_things): + r"""Test the logic that picks things from the server. + + Note that ``_pick_things`` depends only on the ``thing_type`` of the connection, + not on whether it's optional or a mapping. Those are dealt with in ``connect``\ . + """ + attr = ThingWithManyConnections.single_no_default + + def picked_names(things, target): + return {t.name for t in attr._pick_things(things, target)} + + # If the target is None, we always get an empty list. + for names in [[], ["thing1_a"], ["thing1_a", "thing1_b"]]: + assert picked_names(dummy_things(names), None) == set() + + # If there are no other Things, picking by class returns nothing. + assert picked_names({}, ...) == set() + + # If there are other Things, they should be filtered by type. + for names1 in [[], ["thing1_a"], ["thing1_a", "thing1_b"]]: + for names2 in [[], ["thing2_a"], ["thing2_a", "thing2_b"]]: + mixed_things = { + **dummy_things(names1, Dummy1), + **dummy_things(names2, Dummy2), + } + assert picked_names(mixed_things, ...) == set(names1) + + # If a string is specified, it works when it exists and it's the right type. + for target in ["thing1_a", "thing1_b"]: + assert picked_names(mixed_things, target) == {target} + # If a sequence of strings is specified, it should also check existence and type. + # The targets below all exist and have the right type. + for target in [[], ["thing1_a"], ["thing1_a", "thing1_b"]]: + assert picked_names(mixed_things, target) == set(target) + # Any iterable will do - a set is not a sequence, but it is an iterable. + # This checks sets are OK as well. + for target in [set(), {"thing1_a"}, {"thing1_a", "thing1_b"}]: + assert picked_names(mixed_things, target) == target + + # Check for the error if we specify the wrong type (for string and sequence) + # Note that only one thing of the wrong type will still cause the error. + for target in ["thing2_a", ["thing2_a"], ["thing1_a", "thing2_a"]]: + with pytest.raises(ThingSlotError) as excinfo: + picked_names(mixed_things, target) + assert "wrong type" in str(excinfo.value) + + # Check for a KeyError if we specify a missing Thing. This is converted to + # a ThingSlotError by `connect`. + for target in ["something_else", {"thing1_a", "something_else"}]: + with pytest.raises(KeyError): + picked_names(mixed_things, target) + + # Check for a TypeError if the target is the wrong type. + with pytest.raises(TypeError): + picked_names(mixed_things, True) + + +def test_connect(mixed_things): + """Test connecting different attributes produces the right result""" + cls = ThingWithManyConnections # This is just to save typing! + + # A default of None means no things should be returned by default + # This is OK for optional connections and mappings, but not for + # connections typed as a Thing: these must always have a value. + for names in [set(), {"thing_a"}, {"thing_a", "thing_b"}]: + obj = cls() + cls.optional_default_none.connect(obj, dummy_things(names)) + assert obj.optional_default_none is None + cls.multiple_default_none.connect(obj, dummy_things(names)) + assert len(obj.multiple_default_none) == 0 + # single should fail, as it requires a Thing + with pytest.raises(ThingSlotError) as excinfo: + cls.single_default_none.connect(obj, dummy_things(names)) + assert "must be set" in str(excinfo.value) + + # We should be able to override this by giving names. + # Note that a sequence with one element and a single string are equivalent. + for target in ["thing1_a", ["thing1_a"]]: + obj = cls() + cls.single_default_none.connect(obj, mixed_things, target) + assert obj.single_default_none.name == "thing1_a" + cls.optional_default_none.connect(obj, mixed_things, target) + assert obj.optional_default_none.name == "thing1_a" + cls.multiple_default_none.connect(obj, mixed_things, target) + assert isinstance(obj.multiple_default_none, Mapping) + assert set(obj.multiple_default_none.keys()) == {"thing1_a"} + + # A default of `...` (i.e. no default) picks by class. + # Different types have different constraints on how many are allowed. + + # If there are no matching Things, optional and multiple are OK, + # but a single connection fails, as it can't be None. + no_matches = {n: Dummy2(n) for n in ["one", "two"]} + obj = cls() + with pytest.raises(ThingSlotError) as excinfo: + cls.single_no_default.connect(obj, no_matches) + assert "no matching Thing" in str(excinfo.value) + cls.optional_no_default.connect(obj, no_matches) + assert obj.optional_no_default is None + cls.multiple_no_default.connect(obj, no_matches) + assert obj.multiple_no_default == {} + + # If there's exactly one matching Thing, everything works. + match = Dummy1("three") + one_match = {"three": match, **no_matches} + obj = cls() + cls.single_no_default.connect(obj, one_match) + assert obj.single_no_default is match + cls.optional_no_default.connect(obj, one_match) + assert obj.optional_no_default is match + cls.multiple_no_default.connect(obj, one_match) + assert obj.multiple_no_default == {"three": match} + + # If we have more than one match, only the multiple connection + # is OK. + match2 = Dummy1("four") + two_matches = {"four": match2, **one_match} + obj = cls() + with pytest.raises(ThingSlotError) as excinfo: + cls.single_no_default.connect(obj, two_matches) + assert "multiple Things" in str(excinfo.value) + assert "Things by type" in str(excinfo.value) + with pytest.raises(ThingSlotError) as excinfo: + cls.optional_no_default.connect(obj, two_matches) + assert "multiple Things" in str(excinfo.value) + assert "Things by type" in str(excinfo.value) + cls.multiple_no_default.connect(obj, two_matches) + assert obj.multiple_no_default == {"three": match, "four": match2} + + # _pick_things raises KeyErrors for invalid names. + # Check KeyErrors are turned back into ThingSlotErrors + obj = cls() + with pytest.raises(ThingSlotError) as excinfo: + cls.single_default_str.connect(obj, mixed_things) + assert "not the name of a Thing" in str(excinfo.value) + assert f"{obj.name}.single_default_str" in str(excinfo.value) + assert "not configured, and used the default" in str(excinfo.value) + # The error message changes if a target is specified. + obj = cls() + with pytest.raises(ThingSlotError) as excinfo: + cls.single_default_str.connect(obj, mixed_things, "missing") + assert "not the name of a Thing" in str(excinfo.value) + assert f"{obj.name}.single_default_str" in str(excinfo.value) + assert "configured to connect to 'missing'" in str(excinfo.value) + + +def test_readonly(): + """Test that thing connections are read-only.""" + obj = ThingWithManyConnections() + with pytest.raises(AttributeError, match="read-only"): + obj.single_default_none = Dummy("name") + + +def test_referenceerror(): + """Check an error is raised by premature deletion.""" + obj = ThingWithManyConnections() + things = {"name": Dummy1("name")} + ThingWithManyConnections.single_no_default.connect(obj, things) + del things + gc.collect() + with pytest.raises(ReferenceError): + _ = obj.single_no_default + + +# The tests below use real Things and a real ThingServer to do more +# realistic tests. These are not as exhaustive as the tests above, +# but I think there's no harm in taking both approaches. +def test_type_analysis_thingone(): + """Check the correct properties are inferred from the type hints.""" + assert ThingOne.other_thing.is_optional is False + assert ThingOne.other_thing.is_mapping is False + assert ThingOne.other_thing.thing_type == (ThingTwo,) + + assert ThingOne.n_things.is_optional is False + assert ThingOne.n_things.is_mapping is True + assert ThingOne.n_things.thing_type == (ThingThree,) + + assert ThingOne.optional_thing.is_optional is True + assert ThingOne.optional_thing.is_mapping is False + assert ThingOne.optional_thing.thing_type == (ThingThree,) + + +CONNECTIONS = { + "thing_one": {"other_thing": "thing_two"}, + "thing_two": {"other_thing": "thing_one"}, +} + + +@pytest.mark.parametrize( + ("cls_1", "cls_2", "connections"), + [ + (ThingOne, ThingTwo, {}), + (ThingOne, ThingTwo, CONNECTIONS), + (ThingN, ThingN, CONNECTIONS), + ], +) +def test_circular_connection(cls_1, cls_2, connections) -> None: + """Check that two things can connect to each other. + + Note that this test includes a circular dependency, which is fine. + No checks are made for infinite loops: that's up to the author of the + Thing classes. Circular dependencies should not cause any problems for + the LabThings server. + """ + server = lt.ThingServer( + things={ + "thing_one": lt.ThingConfig( + cls=cls_1, thing_slots=connections.get("thing_one", {}) + ), + "thing_two": lt.ThingConfig( + cls=cls_2, thing_slots=connections.get("thing_two", {}) + ), + } + ) + things = [server.things[n] for n in ["thing_one", "thing_two"]] + + with TestClient(server.app) as _: + # The things should be connected as the server is now running + for thing, other in zip(things, reversed(things), strict=True): + assert thing.other_thing is other + + +@pytest.mark.parametrize( + ("connections", "error"), + [ + ({}, "must be set"), + ({"thing_one": {"other_thing": "non_thing"}}, "not the name of a Thing"), + ({"thing_one": {"other_thing": "thing_three"}}, "wrong type"), + ( + { + "thing_one": {"other_thing": "thing_one"}, + "thing_two": {"other_thing": "thing_one"}, + }, + None, + ), + ], +) +def test_connections_none_default(connections, error): + """Check error conditions for a connection with a default of None. + + Note that we only catch the first error - that's why we only need + to specify connections for 'thing_two' in the last case - because + that's the only one where 'thing_one' connects successfully. + """ + things = { + "thing_one": lt.ThingConfig( + cls=ThingN, thing_slots=connections.get("thing_one", {}) + ), + "thing_two": lt.ThingConfig( + cls=ThingN, thing_slots=connections.get("thing_two", {}) + ), + "thing_three": lt.ThingConfig( + cls=ThingThree, thing_slots=connections.get("thing_three", {}) + ), + } + + if error is None: + server = lt.ThingServer(things) + with TestClient(server.app): + thing_one = server.things["thing_one"] + assert isinstance(thing_one, ThingN) + assert thing_one.other_thing is thing_one + return + + with pytest.raises(ThingSlotError, match=error): + server = lt.ThingServer(things) + + +def test_optional_and_empty(): + """Check that an optional or mapping connection can be None/empty.""" + server = lt.ThingServer({"thing_one": ThingOne, "thing_two": ThingTwo}) + + with TestClient(server.app): + thing_one = server.things["thing_one"] + assert isinstance(thing_one, ThingOne) + assert thing_one.optional_thing is None + assert len(thing_one.n_things) == 0 + + +def test_mapping_and_multiple(): + """Check that a mapping connection can pick up several Things. + + This also tests the expected error if multiple things match a + single connection. + """ + things = { + "thing_one": ThingOne, + "thing_two": ThingTwo, + "thing_3": ThingThree, + "thing_4": ThingThree, + "thing_5": ThingThree, + } + # We can't set up a server like this, because + # thing_one.optional_thing will match multiple ThingThree instances. + with pytest.raises(ThingSlotError, match="multiple Things"): + server = lt.ThingServer(things) + + # Set optional thing to one specific name and it will start OK. + things["thing_one"] = lt.ThingConfig( + cls=ThingOne, + thing_slots={"optional_thing": "thing_3"}, + ) + server = lt.ThingServer(things) + with TestClient(server.app): + thing_one = server.things["thing_one"] + assert isinstance(thing_one, ThingOne) + assert thing_one.optional_thing is not None + assert thing_one.optional_thing.name == "thing_3" + assert set(thing_one.n_things.keys()) == {f"thing_{i + 3}" for i in range(3)} diff --git a/tests/test_websocket.py b/tests/test_websocket.py index 00d7947a..41e5ee38 100644 --- a/tests/test_websocket.py +++ b/tests/test_websocket.py @@ -55,8 +55,7 @@ def cancel_myself(self): @pytest.fixture def server(): """Create a server, and add a MyThing test Thing to it.""" - server = lt.ThingServer() - server.add_thing("thing", ThingWithProperties) + server = lt.ThingServer({"thing": ThingWithProperties}) return server