Remove the file_manager module

.coveragerc

@@ -3,3 +3,8 @@ concurrency = multiprocessing, thread
 parallel = true
 sigterm = true
 omit = tests/**/*.py, docs/**/*.py
+
+[report]
+exclude_lines = 
+    pragma: no cover
+    if TYPE_CHECKING:

src/labthings_fastapi/actions/__init__.py

@@ -8,13 +8,11 @@
 from typing import TYPE_CHECKING
 import weakref
 from fastapi import FastAPI, HTTPException, Request
-from fastapi.responses import FileResponse
 from pydantic import BaseModel
 
 from ..utilities import model_to_dict
 from ..utilities.introspection import EmptyInput
 from ..thing_description.model import LinkElement
-from ..file_manager import FileManager
 from .invocation_model import InvocationModel, InvocationStatus
 from ..dependencies.invocation import (
     CancelHook,
@@ -69,10 +67,6 @@
         self.retention_time = action.retention_time
         self.expiry_time: Optional[datetime.datetime] = None
 
-        # This is added post-hoc by the FastAPI endpoint, in
-        # `ActionDescriptor.add_to_fastapi`
-        self._file_manager: Optional[FileManager] = None
-
         # Private state properties
         self._status_lock = Lock()  # This Lock protects properties below
         self._status: InvocationStatus = InvocationStatus.PENDING  # Task status
@@ -143,13 +137,11 @@
         if request:
             href = str(request.url_for("action_invocation", id=self.id))
         else:
             href = f"{ACTION_INVOCATIONS_PATH}/{self.id}"
         links = [
             LinkElement(rel="self", href=href),
             LinkElement(rel="output", href=href + "/output"),
         ]
-        if self._file_manager:
-            links += self._file_manager.links(href)
         return self.action.invocation_model(
             status=self.status,
             id=self.id,
@@ -197,13 +189,13 @@
             with self._status_lock:
                 self._status = InvocationStatus.CANCELLED
                 self.action.emit_changed_event(self.thing, self._status)
         except Exception as e:  # skipcq: PYL-W0703
             logger.exception(e)
             with self._status_lock:
                 self._status = InvocationStatus.ERROR
                 self._exception = e
                 self.action.emit_changed_event(self.thing, self._status)
             raise e
         finally:
             with self._status_lock:
                 self._end_time = datetime.datetime.now()
@@ -286,8 +278,8 @@
 
     def get_invocation(self, id: uuid.UUID) -> Invocation:
         """Retrieve an invocation by ID"""
         with self._invocations_lock:
             return self._invocations[id]
 
     def list_invocations(
         self,
@@ -363,13 +355,13 @@
             with self._invocations_lock:
                 try:
                     invocation: Any = self._invocations[id]
                 except KeyError:
                     raise HTTPException(
                         status_code=404,
                         detail="No action invocation found with ID {id}",
                     )
                 if not invocation.output:
                     raise HTTPException(
                         status_code=503,
                         detail="No result is available for this invocation",
                     )
@@ -377,7 +369,7 @@
                     invocation.output.response
                 ):
                     # TODO: honour "accept" header
                     return invocation.output.response()
                 return invocation.output
 
         @app.delete(
@@ -396,8 +388,8 @@
             with self._invocations_lock:
                 try:
                     invocation: Any = self._invocations[id]
                 except KeyError:
                     raise HTTPException(
                         status_code=404,
                         detail="No action invocation found with ID {id}",
                     )
@@ -413,50 +405,3 @@
                         ),
                     )
                 invocation.cancel()
-
-        @app.get(
-            ACTION_INVOCATIONS_PATH + "/{id}/files",
-            responses={
-                404: {"description": "Invocation ID not found"},
-                503: {"description": "No files are available for this invocation"},
-            },
-        )
-        def action_invocation_files(id: uuid.UUID) -> list[str]:
-            with self._invocations_lock:
-                try:
-                    invocation: Any = self._invocations[id]
-                except KeyError:
-                    raise HTTPException(
-                        status_code=404,
-                        detail="No action invocation found with ID {id}",
-                    )
-                if not invocation._file_manager:
-                    raise HTTPException(
-                        status_code=503,
-                        detail="No files are available for this invocation",
-                    )
-                return invocation._file_manager.filenames
-
-        @app.get(
-            ACTION_INVOCATIONS_PATH + "/{id}/files/{filename}",
-            response_class=FileResponse,
-            responses={
-                404: {"description": "Invocation ID not found, or file not found"},
-                503: {"description": "No files are available for this invocation"},
-            },
-        )
-        def action_invocation_file(id: uuid.UUID, filename: str):
-            with self._invocations_lock:
-                try:
-                    invocation: Any = self._invocations[id]
-                except KeyError:
-                    raise HTTPException(
-                        status_code=404,
-                        detail="No action invocation found with ID {id}",
-                    )
-                if not invocation._file_manager:
-                    raise HTTPException(
-                        status_code=503,
-                        detail="No files are available for this invocation",
-                    )
-                return FileResponse(invocation._file_manager.path(filename))

src/labthings_fastapi/descriptors/action.py

@@ -142,18 +142,18 @@
         try:
             runner = get_blocking_portal(obj)
             if not runner:
                 thing_name = obj.__class__.__name__
                 msg = (
                     f"Cannot emit action changed event. Is {thing_name} connected to "
                     "a running server?"
                 )
                 raise NotConnectedToServerError(msg)
             runner.start_task_soon(
                 self.emit_changed_event_async,
                 obj,
                 status,
             )
         except Exception:
             # TODO: in the unit test, the get_blockint_port throws exception
             ...
 
@@ -186,22 +186,16 @@
             background_tasks: BackgroundTasks,
             **dependencies,
         ):
-            try:
-                action = action_manager.invoke_action(
-                    action=self,
-                    thing=thing,
-                    input=body,
-                    dependencies=dependencies,
-                    id=id,
-                    cancel_hook=cancel_hook,
-                )
-                background_tasks.add_task(action_manager.expire_invocations)
-                return action.response(request=request)
-            finally:
-                try:
-                    action._file_manager = request.state.file_manager
-                except AttributeError:
-                    pass  # This probably means there was no FileManager created.
+            action = action_manager.invoke_action(
+                action=self,
+                thing=thing,
+                input=body,
+                dependencies=dependencies,
+                id=id,
+                cancel_hook=cancel_hook,
+            )
+            background_tasks.add_task(action_manager.expire_invocations)
+            return action.response(request=request)
 
         if issubclass(self.input_model, EmptyInput):
             annotation = Body(default_factory=StrictEmptyInput)

tests/test_action_manager.py

@@ -4,6 +4,7 @@
 from temp_client import poll_task
 import time
 import labthings_fastapi as lt
+from labthings_fastapi.actions import ACTION_INVOCATIONS_PATH
 
 
 class TestThing(lt.Thing):
@@ -22,16 +23,20 @@ def increment_counter(self):
 server.add_thing(thing, "/thing")
 
 
-def action_partial(client: TestClient, url: str):
-    def run(payload=None):
-        r = client.post(url, json=payload)
-        assert r.status_code in (200, 201)
-        return poll_task(client, r.json())
+def test_action_expires():
+    """Check the action is removed from the server
 
-    return run
+    We've set the retention period to be very short, so the action
+    should not be retrievable after some time has elapsed.
 
+    This test checks that actions do indeed get removed.
 
-def test_expiry():
+    Note that the code that expires actions runs whenever a new
+    action is started. That's why we need to invoke the action twice:
+    the second invocation runs the code that deletes the first one.
+    This behaviour might change in the future, making the second run
+    unnecessary.
+    """
     with TestClient(server.app) as client:
         before_value = client.get("/thing/counter").json()
         r = client.post("/thing/increment_counter")
@@ -42,5 +47,24 @@ def test_expiry():
         after_value = client.get("/thing/counter").json()
         assert after_value == before_value + 2
         invocation["status"] = "running"  # Force an extra poll
+        # When the second action runs, the first one should expire
+        # so polling it again should give a 404.
         with pytest.raises(httpx.HTTPStatusError):
             poll_task(client, invocation)
+
+
+def test_actions_list():
+    """Check that the /action_invocations/ path works.
+
+    The /action_invocations/ path should return a list of invocation
+    objects (a representation of each action that's been run recently).
+
+    It's implemented in `ActionManager.list_all_invocations`.
+    """
+    with TestClient(server.app) as client:
+        r = client.post("/thing/increment_counter")
+        invocation = poll_task(client, r.json())
+        r2 = client.get(ACTION_INVOCATIONS_PATH)
+        r2.raise_for_status()
+        invocations = r2.json()
+        assert invocations == [invocation]

tests/test_dependencies.py

@@ -6,7 +6,6 @@
 
 from fastapi import Depends, FastAPI, Request
 from labthings_fastapi.deps import InvocationID
-from labthings_fastapi.file_manager import FileManagerDep
 from fastapi.testclient import TestClient
 from module_with_deps import FancyIDDep
 
@@ -55,17 +54,3 @@ def endpoint(id: DepClass = Depends()) -> bool:
         assert r.status_code == 200
         invocation = r.json()
         assert invocation is True
-
-
-def test_file_manager():
-    app = FastAPI()
-
-    @app.post("/invoke_with_file")
-    def invoke_with_file(
-        file_manager: FileManagerDep,
-    ) -> dict[str, str]:
-        return {"directory": str(file_manager.directory)}
-
-    with TestClient(app) as client:
-        r = client.post("/invoke_with_file")
-        assert r.status_code == 200

tests/test_dependencies_2.py

@@ -22,7 +22,6 @@
 from fastapi.testclient import TestClient
 from module_with_deps import FancyIDDep, FancyID, ClassDependsOnFancyID
 import labthings_fastapi as lt
-from labthings_fastapi.file_manager import FileManager
 
 
 def test_dep_from_module():
@@ -147,17 +146,3 @@ def endpoint(id: lt.deps.InvocationID) -> bool:
     with TestClient(app) as client:
         r = client.post("/endpoint")
         assert r.status_code == 200
-
-
-def test_filemanager_dep():
-    """Test out our FileManager class as a dependency"""
-    app = FastAPI()
-
-    @app.post("/endpoint")
-    def endpoint(fm: Annotated[FileManager, Depends()]) -> str:
-        return f"Saving to {fm.directory}"
-
-    with TestClient(app) as client:
-        r = client.post("/endpoint")
-        assert r.status_code == 200
-        assert r.json().startswith("Saving to ")