diff --git a/docs/source/quickstart/counter.py b/docs/source/quickstart/counter.py index efe22030..10f8a827 100644 --- a/docs/source/quickstart/counter.py +++ b/docs/source/quickstart/counter.py @@ -18,7 +18,7 @@ def increment_counter(self) -> None: @lt.thing_action def slowly_increase_counter(self) -> None: """Increment the counter slowly over a minute""" - for i in range(60): + for _i in range(60): time.sleep(1) self.increment_counter() diff --git a/pyproject.toml b/pyproject.toml index 772475e2..052f0a65 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -76,7 +76,7 @@ docstring-code-format = true [tool.ruff.lint] external = ["DOC401", "F824", "DOC101", "DOC103"] # used via flake8/pydoclint -select = ["E4", "E7", "E9", "F", "D", "DOC"] +select = ["B", "E4", "E7", "E9", "F", "D", "DOC"] ignore = [ "D203", # incompatible with D204 "D213", # incompatible with D212 @@ -84,6 +84,9 @@ ignore = [ "DOC201", # doesn't work with sphinx-style docstrings, use flake8/pydoclint "DOC501", # doesn't work with sphinx-style docstrings, use flake8/pydoclint "DOC502", # doesn't work with sphinx-style docstrings, use flake8/pydoclint + "B008", # This disallows function calls in default values. + # FastAPI Depends() breaks this rule, and FastAPI's response is "disable it". + # see https://github.com/fastapi/fastapi/issues/1522 ] preview = true diff --git a/src/labthings_fastapi/actions/__init__.py b/src/labthings_fastapi/actions/__init__.py index 5d51f84a..56e251fa 100644 --- a/src/labthings_fastapi/actions/__init__.py +++ b/src/labthings_fastapi/actions/__init__.py @@ -491,11 +491,11 @@ def action_invocation( try: with self._invocations_lock: return self._invocations[id].response(request=request) - except KeyError: + except KeyError as e: raise HTTPException( status_code=404, detail="No action invocation found with ID {id}", - ) + ) from e @app.get( ACTION_INVOCATIONS_PATH + "/{id}/output", @@ -530,11 +530,11 @@ def action_invocation_output(id: uuid.UUID, _blob_manager: BlobIOContextDep): with self._invocations_lock: try: invocation: Any = self._invocations[id] - except KeyError: + except KeyError as e: raise HTTPException( status_code=404, detail="No action invocation found with ID {id}", - ) + ) from e if not invocation.output: raise HTTPException( status_code=503, @@ -569,11 +569,11 @@ def delete_invocation(id: uuid.UUID) -> None: with self._invocations_lock: try: invocation: Any = self._invocations[id] - except KeyError: + except KeyError as e: raise HTTPException( status_code=404, detail="No action invocation found with ID {id}", - ) + ) from e if invocation.status not in [ InvocationStatus.RUNNING, InvocationStatus.PENDING, diff --git a/src/labthings_fastapi/client/__init__.py b/src/labthings_fastapi/client/__init__.py index c7345869..320c9fd8 100644 --- a/src/labthings_fastapi/client/__init__.py +++ b/src/labthings_fastapi/client/__init__.py @@ -57,8 +57,8 @@ def _get_link(obj: dict, rel: str) -> Mapping: raise ObjectHasNoLinksError(f"Can't find any links on {obj}.") try: return next(link for link in obj["links"] if link["rel"] == rel) - except StopIteration: - raise KeyError(f"No link was found with rel='{rel}' on {obj}.") + except StopIteration as e: + raise KeyError(f"No link was found with rel='{rel}' on {obj}.") from e def invocation_href(invocation: dict) -> str: diff --git a/src/labthings_fastapi/client/in_server.py b/src/labthings_fastapi/client/in_server.py index 2aedf6c6..d959d650 100644 --- a/src/labthings_fastapi/client/in_server.py +++ b/src/labthings_fastapi/client/in_server.py @@ -265,13 +265,24 @@ def direct_thing_client_class( This class may be used as a FastAPI dependency: see :ref:`things_from_things`. """ - def init_proxy(self, request: Request, **dependencies: Mapping[str, Any]): - f"""A client for {thing_class} at {thing_path}""" + def init_proxy( + self: DirectThingClient, request: Request, **dependencies: Mapping[str, Any] + ): + r"""Initialise a DirectThingClient (this docstring will be replaced). + + :param self: The DirectThingClient instance we're initialising. + :param request: a FastAPI Request option (will be supplied by FastAPI). + :param \**dependencies: Other keyword arguments will be saved as + dependencies. FastAPI will look at the signature (which we will + manipulate below) to determine these. + """ # NB this definition isimportant, as we must modify its signature. # Inheriting __init__ means we'll accidentally modify the signature # of `DirectThingClient` with bad results. DirectThingClient.__init__(self, request, **dependencies) + init_proxy.__doc__ = f"""Initialise a client for {thing_class} at {thing_path}""" + # Using a class definition gets confused by the scope of the function # arguments - this is equivalent to a class definition but all the # arguments are evaluated in the right scope. diff --git a/src/labthings_fastapi/example_things/__init__.py b/src/labthings_fastapi/example_things/__init__.py index 3ed50647..def9bb57 100644 --- a/src/labthings_fastapi/example_things/__init__.py +++ b/src/labthings_fastapi/example_things/__init__.py @@ -90,7 +90,7 @@ def slowly_increase_counter(self, increments: int = 60, delay: float = 1): :param increments: how many times to increment. :param delay: the wait time between increments. """ - for i in range(increments): + for _i in range(increments): time.sleep(delay) self.increment_counter() diff --git a/src/labthings_fastapi/outputs/blob.py b/src/labthings_fastapi/outputs/blob.py index 8043df23..420ad7f8 100644 --- a/src/labthings_fastapi/outputs/blob.py +++ b/src/labthings_fastapi/outputs/blob.py @@ -364,11 +364,11 @@ def retrieve_data(self) -> Self: url_to_blobdata = url_to_blobdata_ctx.get() self._data = url_to_blobdata(self.href) self.href = "blob://local" - except LookupError: + except LookupError as e: raise LookupError( "Blobs may only be created from URLs passed in over HTTP." f"The URL in question was {self.href}." - ) + ) from e return self @model_serializer(mode="plain", when_used="always") @@ -398,11 +398,11 @@ def to_dict(self) -> Mapping[str, str]: blobdata_to_url = blobdata_to_url_ctx.get() # MyPy seems to miss that `self.data` is a property, hence the ignore href = blobdata_to_url(self.data) # type: ignore[arg-type] - except LookupError: + except LookupError as e: raise LookupError( "Blobs may only be serialised inside the " "context created by BlobIOContextDep." - ) + ) from e else: href = self.href return { diff --git a/src/labthings_fastapi/server/__init__.py b/src/labthings_fastapi/server/__init__.py index 1235d7fc..0ef9abd9 100644 --- a/src/labthings_fastapi/server/__init__.py +++ b/src/labthings_fastapi/server/__init__.py @@ -210,7 +210,7 @@ async def lifespan(self, app: FastAPI) -> AsyncGenerator[None]: for thing in self.things.values(): await stack.enter_async_context(thing) yield - for name, thing in self.things.items(): + for _name, thing in self.things.items(): # Remove the blocking portal - the event loop is about to stop. thing._labthings_blocking_portal = None @@ -284,9 +284,8 @@ def server_from_config(config: dict) -> ThingServer: except ImportError as e: raise ImportError( f"Could not import {thing['class']}, which was " - f"specified as the class for {path}. The error is " - f"printed below:\n\n{e}" - ) + f"specified as the class for {path}." + ) from e instance = cls(*thing.get("args", {}), **thing.get("kwargs", {})) assert isinstance(instance, Thing), f"{thing['class']} is not a Thing" server.add_thing(instance, path) diff --git a/src/labthings_fastapi/server/cli.py b/src/labthings_fastapi/server/cli.py index 068490b9..5a1aefbf 100644 --- a/src/labthings_fastapi/server/cli.py +++ b/src/labthings_fastapi/server/cli.py @@ -96,8 +96,10 @@ def config_from_args(args: Namespace) -> dict: try: with open(args.config) as f: config = json.load(f) - except FileNotFoundError: - raise FileNotFoundError(f"Could not find configuration file {args.config}") + except FileNotFoundError as e: + raise FileNotFoundError( + f"Could not find configuration file {args.config}" + ) from e else: config = {} if args.json: diff --git a/src/labthings_fastapi/thing_description/__init__.py b/src/labthings_fastapi/thing_description/__init__.py index 845167a1..a06497f1 100644 --- a/src/labthings_fastapi/thing_description/__init__.py +++ b/src/labthings_fastapi/thing_description/__init__.py @@ -75,9 +75,8 @@ def look_up_reference(reference: str, d: JSONSchema) -> JSONSchema: return resolved except KeyError as ke: raise KeyError( - f"The JSON reference {reference} was not found in the schema " - f"(original error {ke})." - ) + f"The JSON reference {reference} was not found in the schema." + ) from ke def is_an_object(d: JSONSchema) -> bool: diff --git a/src/labthings_fastapi/utilities/object_reference_to_object.py b/src/labthings_fastapi/utilities/object_reference_to_object.py index d95e543c..72ddceb3 100644 --- a/src/labthings_fastapi/utilities/object_reference_to_object.py +++ b/src/labthings_fastapi/utilities/object_reference_to_object.py @@ -26,9 +26,9 @@ def object_reference_to_object(object_reference: str) -> Any: for attr in qualname.split("."): try: obj = getattr(obj, attr) - except AttributeError: + 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/module_with_deps.py b/tests/module_with_deps.py index a7f86fc0..95d7a9c7 100644 --- a/tests/module_with_deps.py +++ b/tests/module_with_deps.py @@ -1,11 +1,28 @@ -from __future__ import annotations +"""A module for testing dependencies. + +This module provides some classes that are used as dependencies by unit tests. +Note that `from __future__ import annotations` is not used here. If it is used, +we would need to add the following to the classes: + +.. code-block:: python + + class Whatever: + __globals__ = globals() # "bake in" globals so dependency injection works + +This relates to the way FastAPI resolves annotations to objects. There's an issue +thread that discusses the work-around above explicitly, but it's part of a bigger +issue discussed here: + +https://github.com/pydantic/pydantic/issues/2678 + +""" + +from dataclasses import dataclass from typing import Annotated from fastapi import Depends, Request class FancyID: - __globals__ = globals() # "bake in" globals so dependency injection works - def __init__(self, r: Request): self.id = 1234 @@ -13,8 +30,8 @@ def __init__(self, r: Request): FancyIDDep = Annotated[FancyID, Depends()] +@dataclass class ClassDependsOnFancyID: - __globals__ = globals() # "bake in" globals so dependency injection works + """A dataclass that will request a FancyID when used as a Dependency.""" - def __init__(self, sub: Annotated[FancyID, Depends()]): - self.sub = sub + sub: FancyIDDep diff --git a/tests/test_action_cancel.py b/tests/test_action_cancel.py index 7ec923cf..5e7c5742 100644 --- a/tests/test_action_cancel.py +++ b/tests/test_action_cancel.py @@ -20,7 +20,7 @@ class CancellableCountingThing(lt.Thing): @lt.thing_action def count_slowly(self, cancel: lt.deps.CancelHook, n: int = 10): - for i in range(n): + for _i in range(n): try: cancel.sleep(0.1) except lt.exceptions.InvocationCancelledError as e: @@ -35,7 +35,7 @@ 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): + for _i in range(n): try: cancel.sleep(0.1) except lt.exceptions.InvocationCancelledError: @@ -52,7 +52,7 @@ def count_and_only_cancel_if_asked_twice( """ cancelled_once = False counting_increment = 1 - for i in range(n): + for _i in range(n): try: cancel.sleep(0.1) except lt.exceptions.InvocationCancelledError as e: diff --git a/tests/test_action_logging.py b/tests/test_action_logging.py index 86fecf9e..c526e574 100644 --- a/tests/test_action_logging.py +++ b/tests/test_action_logging.py @@ -31,7 +31,9 @@ def test_invocation_logging(caplog): invocation = poll_task(client, r.json()) assert invocation["status"] == "completed" assert len(invocation["log"]) == len(ThingOne.LOG_MESSAGES) - for expected, entry in zip(ThingOne.LOG_MESSAGES, invocation["log"]): + for expected, entry in zip( + ThingOne.LOG_MESSAGES, invocation["log"], strict=True + ): assert entry["message"] == expected diff --git a/tests/test_base_descriptor.py b/tests/test_base_descriptor.py index e283d438..34e37637 100644 --- a/tests/test_base_descriptor.py +++ b/tests/test_base_descriptor.py @@ -161,7 +161,7 @@ def test_basedescriptor_orphaned(): """Check the right error is raised if we ask for the name outside a class.""" prop = MockProperty() with pytest.raises(DescriptorNotAddedToClassError): - prop.name + _ = prop.name def test_basedescriptor_fallback(): @@ -185,7 +185,7 @@ def test_basedescriptor_get(): assert isinstance(Example.my_property, MockProperty) with pytest.raises(NotImplementedError): # BaseDescriptor requires `instance_get` to be overridden. - e.base_descriptor + _ = e.base_descriptor class MockFunctionalProperty(MockProperty): diff --git a/tests/test_dependencies.py b/tests/test_dependencies.py index a8195d86..f33d634a 100644 --- a/tests/test_dependencies.py +++ b/tests/test_dependencies.py @@ -4,6 +4,7 @@ for actions. """ +from dataclasses import dataclass from fastapi import Depends, FastAPI, Request from labthings_fastapi.deps import InvocationID from fastapi.testclient import TestClient @@ -41,9 +42,15 @@ def test_dependency_needing_request(): """Test a dependency that requires Request object""" app = FastAPI() + @dataclass class DepClass: - def __init__(self, sub: Request): - self.sub = sub + r"""A class that has a dependency in its __init__. + + This is a dataclass, so __init__ is generated automatically and + will have an argument `sub` with type `Request`\ . + """ + + sub: Request @app.post("/dep") def endpoint(id: DepClass = Depends()) -> bool: diff --git a/tests/test_dependencies_2.py b/tests/test_dependencies_2.py index 8702ead0..5f682d68 100644 --- a/tests/test_dependencies_2.py +++ b/tests/test_dependencies_2.py @@ -17,6 +17,7 @@ mitigation against something changing in the future. """ +from dataclasses import dataclass from typing import Annotated from fastapi import Depends, FastAPI from fastapi.testclient import TestClient @@ -45,6 +46,8 @@ def test_dep_from_module_with_subdep(): @app.post("/endpoint") def endpoint(id: Annotated[ClassDependsOnFancyID, Depends()]) -> bool: + # Verify that the dependency is supplied, including its sub-dependency + assert id.sub.id == 1234 return True with TestClient(app) as client: @@ -101,25 +104,44 @@ def endpoint(id: DepClass = Depends()) -> bool: def test_class_dep_with_subdep(): - """Add an endpoint that uses a dependency class with sub-dependency""" + """Add an endpoint that uses a dependency class with sub-dependency. + + We do this twice, using a regular class and also a dataclass. + """ app = FastAPI() class SubDepClass: pass - class DepClass: + class DepClass: # noqa B903 + """A regular class that has sub-dependencies via __init__. + + Note that this could be a dataclass, but we want to check both + dataclasses and normal classes.""" + def __init__(self, sub: Annotated[SubDepClass, Depends()]): self.sub = sub @app.post("/dep") def endpoint(id: DepClass = Depends()) -> bool: + assert isinstance(id.sub, SubDepClass) + return True + + @dataclass + class DepDataclass: + sub: Annotated[SubDepClass, Depends()] + + @app.post("/dep2") + def endpoint2(dep: Annotated[DepDataclass, Depends()]): + assert isinstance(dep.sub, SubDepClass) return True with TestClient(app) as client: - r = client.post("/dep") - assert r.status_code == 200 - invocation = r.json() - assert invocation is True + for url in ["/dep", "/dep2"]: + r = client.post(url) + assert r.status_code == 200 + invocation = r.json() + assert invocation is True def test_invocation_id(): diff --git a/tests/test_example_thing.py b/tests/test_example_thing.py index ab6e454c..14ebe23f 100644 --- a/tests/test_example_thing.py +++ b/tests/test_example_thing.py @@ -49,13 +49,13 @@ def test_thing_with_broken_affordances(): def test_thing_that_cannot_instantiate(): - with pytest.raises(Exception): + with pytest.raises(RuntimeError): ThingThatCantInstantiate() def test_thing_that_cannot_start(): thing = ThingThatCantStart() assert isinstance(thing, ThingThatCantStart) - with pytest.raises(Exception): + with pytest.raises(RuntimeError): with thing: pass diff --git a/tests/test_property.py b/tests/test_property.py index e15ab1be..aad5db5b 100644 --- a/tests/test_property.py +++ b/tests/test_property.py @@ -157,9 +157,9 @@ def test_baseproperty_type_and_model(): # By default, we have no type so `.type` errors. with pytest.raises(tp.MissingTypeError): - prop.value_type + _ = prop.value_type with pytest.raises(tp.MissingTypeError): - prop.model + _ = prop.model # Once _type is set, these should both work. prop._type = str | None diff --git a/tests/test_server_cli.py b/tests/test_server_cli.py index dce81b0e..e2d63f15 100644 --- a/tests/test_server_cli.py +++ b/tests/test_server_cli.py @@ -14,12 +14,8 @@ def monitored_target(target, conn, *args, **kwargs): """Monitor stdout and exceptions from a function""" # The lines below copy stdout messages to a pipe # which allows us to monitor STDOUT and STDERR - for output, name in [(sys.stdout, "stdout"), (sys.stderr, "stderr")]: - - def write_wrapper(message): - conn.send((name, message)) - - output.write = write_wrapper + sys.stdout.write = lambda message: conn.send(("stdout", message)) + sys.stderr.write = lambda message: conn.send(("stderr", message)) try: ret = target(*args, **kwargs) @@ -44,8 +40,10 @@ def __init__(self, target=None, **kwargs): self, target=monitored_target, args=args, **kwargs ) - def run_monitored(self, terminate_outputs=[], timeout=10): + def run_monitored(self, terminate_outputs=None, timeout=10): """Run the process, monitoring stdout and exceptions""" + if terminate_outputs is None: + terminate_outputs = [] self.start() try: while self._pconn.poll(timeout): @@ -84,8 +82,10 @@ def test_server_from_config(): assert isinstance(server, ThingServer) -def check_serve_from_cli(args: list[str] = []): +def check_serve_from_cli(args: list[str] | None = None): """Check we can create a server from the command line""" + if args is None: + args = [] p = MonitoredProcess(target=serve_from_cli, args=(args,)) p.run_monitored(terminate_outputs=["Application startup complete"])