From c2a6f61845a57a4dd4e180f8808d4dcc1ced2cf3 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 17 Jul 2025 09:13:40 +0100 Subject: [PATCH 1/3] Add BlobIOContextDep to ActionDescriptor.list_invocations so context vars are available --- src/labthings_fastapi/descriptors/action.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/labthings_fastapi/descriptors/action.py b/src/labthings_fastapi/descriptors/action.py index 58ed321c..6129775f 100644 --- a/src/labthings_fastapi/descriptors/action.py +++ b/src/labthings_fastapi/descriptors/action.py @@ -339,7 +339,9 @@ def start_action( ), summary=f"All invocations of {self.name}.", ) - def list_invocations(action_manager: ActionManagerContextDep): + def list_invocations( + action_manager: ActionManagerContextDep, _blob_manager: BlobIOContextDep + ): return action_manager.list_invocations(self, thing) def action_affordance( From f8ff0fe30f86780e3a0b0b98d96e5f143452ff66 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 17 Jul 2025 10:20:56 +0100 Subject: [PATCH 2/3] Add unit test that fails without the fix in previous commit --- src/labthings_fastapi/actions/__init__.py | 19 +++++++++++++++++- tests/test_actions.py | 24 +++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/labthings_fastapi/actions/__init__.py b/src/labthings_fastapi/actions/__init__.py index 3cd6324e..98db5889 100644 --- a/src/labthings_fastapi/actions/__init__.py +++ b/src/labthings_fastapi/actions/__init__.py @@ -35,7 +35,7 @@ InvocationCancelledError, invocation_logger, ) -from ..outputs.blob import BlobIOContextDep +from ..outputs.blob import BlobIOContextDep, blobdata_to_url_ctx if TYPE_CHECKING: # We only need these imports for type hints, so this avoids circular imports. @@ -46,6 +46,14 @@ """The API route used to list `.Invocation` objects.""" +class NoBlobManagerError(RuntimeError): + """Raised if an API route accesses Invocation outputs without a BlobIOContextDep. + + Any access to an invocation output must have BlobIOContextDep as a dependency, as + the output may be a blob, and the blob needs this context to resolve its URL. + """ + + class Invocation(Thread): """A Thread subclass that retains output values and tracks progress. @@ -124,6 +132,15 @@ def id(self) -> uuid.UUID: @property def output(self) -> Any: """Return value of the Action. If the Action is still running, returns None.""" + try: + blobdata_to_url_ctx.get() + except LookupError as e: + raise NoBlobManagerError( + "An invocation output has been requested from a api route that " + "doesn't have a BlobIOContextDep dependency. This dependency is needed " + " for blobs to identify their url." + ) from e + with self._status_lock: return self._return_value diff --git a/tests/test_actions.py b/tests/test_actions.py index 26d4edb7..18a5d257 100644 --- a/tests/test_actions.py +++ b/tests/test_actions.py @@ -18,6 +18,30 @@ def run(payload=None): return run +def test_get_action_invocations(): + """Test that running "get" on an action returns a list of invocations.""" + with TestClient(server.app) as client: + # When we start the action has no invocations + invocations_before = client.get("/thing/increment_counter").json() + assert invocations_before == [] + # Start the action + r = client.post("/thing/increment_counter") + assert r.status_code in (200, 201) + # Now it is started, there is a list of 1 dictionary containing the + # invocation information. + invocations_after = client.get("/thing/increment_counter").json() + assert len(invocations_after) == 1 + assert isinstance(invocations_after, list) + assert isinstance(invocations_after[0], dict) + assert "status" in invocations_after[0] + assert "id" in invocations_after[0] + assert "action" in invocations_after[0] + assert "href" in invocations_after[0] + assert "timeStarted" in invocations_after[0] + # Let the task finish before ending the test + poll_task(client, r.json()) + + def test_counter(): with TestClient(server.app) as client: before_value = client.get("/thing/counter").json() From cdccab058313755a2ecb8d2acf51ff662c96eaca Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Sat, 19 Jul 2025 01:05:41 +0100 Subject: [PATCH 3/3] Update docstring for explicit raise message --- src/labthings_fastapi/actions/__init__.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/labthings_fastapi/actions/__init__.py b/src/labthings_fastapi/actions/__init__.py index 98db5889..5d51f84a 100644 --- a/src/labthings_fastapi/actions/__init__.py +++ b/src/labthings_fastapi/actions/__init__.py @@ -131,7 +131,14 @@ def id(self) -> uuid.UUID: @property def output(self) -> Any: - """Return value of the Action. If the Action is still running, returns None.""" + """Return value of the Action. If the Action is still running, returns None. + + :raise NoBlobManagerError: If this is called in a context where the blob + manager context variables are not available. This stops errors being raised + later once the blob is returned and tries to serialise. If the errors + happen during serialisation the stack-trace will not clearly identify + the route with the missing dependency. + """ try: blobdata_to_url_ctx.get() except LookupError as e: