diff --git a/docs/reference/openapi.yaml b/docs/reference/openapi.yaml index e0cd8282d..c17734bbe 100644 --- a/docs/reference/openapi.yaml +++ b/docs/reference/openapi.yaml @@ -236,6 +236,11 @@ components: additionalProperties: false description: Task that will run a plan properties: + args: + description: Positional arguments to plan + items: {} + title: Args + type: array metadata: additionalProperties: true description: Any metadata to apply to all runs within this task @@ -247,7 +252,7 @@ components: type: string params: additionalProperties: true - description: Values for parameters to plan, if any + description: Keyword arguments to plan, if any title: Params type: object required: @@ -278,6 +283,11 @@ components: additionalProperties: false description: Request to run a task with related info properties: + args: + description: Positional arguments to plan + items: {} + title: Args + type: array instrument_session: description: Instrument session associated with this task title: Instrument Session @@ -449,7 +459,7 @@ info: name: Apache 2.0 url: https://www.apache.org/licenses/LICENSE-2.0.html title: BlueAPI Control - version: 1.5.0 + version: 1.6.0 openapi: 3.1.0 paths: /api/v1/devices: diff --git a/example-services b/example-services index 9e0faf7cb..3c0274b24 160000 --- a/example-services +++ b/example-services @@ -1 +1 @@ -Subproject commit 9e0faf7cbea7e40ea63d857f046f61675b35b907 +Subproject commit 3c0274b244d28bc6e197f9e3610355cc7c0cc5d3 diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 81dc83637..8424c3c4e 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -73,6 +73,26 @@ def convert( return super().convert(value, param, ctx) +class PositionalArgType(ParamType): + """CLI input parameter to accept a JSON value as a list of positional args""" + + name = "PositionalArgs" + + def convert( + self, value: Any | None, param: Parameter | None, ctx: Context | None + ) -> Any: + if isinstance(value, str): + try: + args = json.loads(value) + if isinstance(args, list): + return args + self.fail("Positional arguments should be a list of arguments") + except json.JSONDecodeError as jde: + self.fail(f"Argument is not valid JSON: {jde}") + else: + return super().convert(value, param, ctx) + + def is_str_dict(val: Any) -> TypeGuard[TaskParameters]: return isinstance(val, dict) and all(isinstance(k, str) for k in val) @@ -321,6 +341,21 @@ def on_event( @controller.command(name="run") @click.argument("name", type=str) @click.argument("parameters", type=ParametersType(), default={}, required=False) +@click.option( + "-a", + "--args", + type=PositionalArgType(), + default=[], + required=False, + help=textwrap.dedent("""\ + Arguments to be passed to the plan as positional arguments. + + This should be passed as a single string representing a JSON array and + may need quoting in some shells, eg + + --args '[1, 2, "foo"]' + """), +) @click.option( "--ws/--stomp", type=bool, @@ -366,6 +401,7 @@ def run_plan( foreground: bool, ws: bool | None, instrument_session: str, + args: list[Any], parameters: TaskParameters, ) -> None: """Run a plan with parameters @@ -377,7 +413,7 @@ def run_plan( client = cast(BlueapiClient, obj["client"]) task = TaskRequest( - name=name, params=parameters, instrument_session=instrument_session + name=name, args=args, params=parameters, instrument_session=instrument_session ) try: diff --git a/src/blueapi/client/plan_cache.py b/src/blueapi/client/plan_cache.py index 69e64a1ab..acf5b27ee 100644 --- a/src/blueapi/client/plan_cache.py +++ b/src/blueapi/client/plan_cache.py @@ -1,6 +1,7 @@ import logging from typing import Any +from blueapi.client.rest import InvalidParametersError from blueapi.service.model import PlanModel, TaskRequest from blueapi.worker.event import TaskError, TaskResult, TaskStatus @@ -76,14 +77,18 @@ def __call__(self, *args, **kwargs) -> Any: """ req = TaskRequest( name=self.name, - params=self._build_args(*args, **kwargs), + args=list(args), + params=kwargs, instrument_session=self._client.instrument_session, ) - match self._client.run_task(req): - case TaskStatus(result=TaskResult(result=res)): - return res - case TaskStatus(result=TaskError(type=typ, message=msg)): - raise PlanFailedError(typ, msg) + try: + match self._client.run_task(req): + case TaskStatus(result=TaskResult(result=res)): + return res + case TaskStatus(result=TaskError(type=typ, message=msg)): + raise PlanFailedError(typ, msg) + except InvalidParametersError as ipe: + raise TypeError(ipe.message()) from None @property def help_text(self) -> str: @@ -97,47 +102,6 @@ def properties(self) -> dict[str, Any]: def required(self) -> list[str]: return self.model.parameter_schema.get("required", []) - def _build_args(self, *args, **kwargs) -> dict[str, Any]: - """Build a parameter mapping from positional and keyword arguments. - - Positional arguments are assigned to parameters according to their - order in the plan's parameter schema. Keyword arguments are then - added by name. - - Raises: - TypeError: If too many positional arguments, unexpected keyword - arguments, duplicate arguments, or required arguments are - supplied incorrectly. - """ - log.info( - "Building args for %s, using %s and %s", - "[" + ",".join(self.properties) + "]", - args, - kwargs, - ) - - if len(args) > len(self.properties): - raise TypeError(f"{self.name} got too many arguments") - if extra := {k for k in kwargs if k not in self.properties}: - raise TypeError(f"{self.name} got unexpected arguments: {extra}") - - params = {} - # Initially fill parameters using positional args assuming the order - # from the parameter_schema - for req, arg in zip(self.properties, args, strict=False): - params[req] = arg - - # Then append any values given via kwargs - for key, value in kwargs.items(): - # If we've already assumed a positional arg was this value, bail out - if key in params: - raise TypeError(f"{self.name} got multiple values for {key}") - params[key] = value - - if missing := {k for k in self.required if k not in params}: - raise TypeError(f"Missing argument(s) for {missing}") - return params - def __repr__(self) -> str: """Return a signature-like representation of the plan. diff --git a/src/blueapi/client/rest.py b/src/blueapi/client/rest.py index 166620ac2..f07f6b659 100644 --- a/src/blueapi/client/rest.py +++ b/src/blueapi/client/rest.py @@ -114,10 +114,12 @@ def __str__(self) -> str: return f"Missing value for {self.field()!r}" case "extra_forbidden": return f"Unexpected field {self.field()!r}" - case _: + case _ if self.loc[2:]: return ( f"Invalid value {self.input!r} for field {self.field()}: {self.msg}" ) + case _: + return self.msg or "Unknown error" class InvalidParametersError(Exception): diff --git a/src/blueapi/config.py b/src/blueapi/config.py index 9c339c71f..f65dd65f0 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -335,7 +335,7 @@ class ApplicationConfig(BlueapiBaseModel): """ #: API version to publish in OpenAPI schema - REST_API_VERSION: ClassVar[str] = "1.5.0" + REST_API_VERSION: ClassVar[str] = "1.6.0" LICENSE_INFO: ClassVar[dict[str, str]] = { "name": "Apache 2.0", diff --git a/src/blueapi/core/context.py b/src/blueapi/core/context.py index 76ce74e64..f8a96b0d6 100644 --- a/src/blueapi/core/context.py +++ b/src/blueapi/core/context.py @@ -439,7 +439,7 @@ def _type_spec_for_function( Returns: Mapping of {name: (type, default)} to be used by pydantic for deserialising - function arguments + function arguments """ args = signature(func).parameters types = get_type_hints(func) @@ -451,7 +451,16 @@ def _type_spec_for_function( f"Type annotation is required for '{name}' in '{func.__name__}'" ) - no_default = para.default is Parameter.empty + match para.kind: + case Parameter.VAR_POSITIONAL: + arg_type = tuple[arg_type, ...] + case Parameter.VAR_KEYWORD: + arg_type = dict[str, arg_type] + + no_default = para.default is Parameter.empty and para.kind not in ( + Parameter.VAR_POSITIONAL, + Parameter.VAR_KEYWORD, + ) if ( isclass(arg_type) and (issubclass(arg_type, BaseModel) or is_dataclass(arg_type)) @@ -462,9 +471,15 @@ def _type_spec_for_function( info = FieldInfo(default_factory=default_factory) else: _type = self._convert_type(arg_type, no_default) + match para.default: case Parameter.empty: - info = FieldInfo(default_factory=None) + if para.kind is Parameter.VAR_POSITIONAL: + info = FieldInfo(default_factory=tuple) + elif para.kind is Parameter.VAR_KEYWORD: + info = FieldInfo(default_factory=dict) + else: + info = FieldInfo(default_factory=None) case None: info = FieldInfo(default_factory=lambda: None) case _: diff --git a/src/blueapi/service/interface.py b/src/blueapi/service/interface.py index a767c4043..7df474557 100644 --- a/src/blueapi/service/interface.py +++ b/src/blueapi/service/interface.py @@ -220,6 +220,7 @@ def submit_task( ] task = Task( name=task_request.name, + args=task_request.args, params=task_request.params, metadata=metadata, ) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 225413304..2ba310787 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -361,12 +361,17 @@ def submit_task( } for err in e.errors ] - LOGGER.info("Error submitting task: %s - %s", task_request, e) - raise HTTPException( - status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, - detail=errors, - ) from e + except Exception as e: + # Catch all for anything else that can go wrong + errors = [ + {"loc": ["body", "params"], "msg": str(e), "type": "invalid", "input": None} + ] + LOGGER.info("Error submitting task: %s - %s", task_request, e) + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, + detail=errors, + ) @secure_router_v1.delete( diff --git a/src/blueapi/service/model.py b/src/blueapi/service/model.py index dbaa1d965..1b690d5c0 100644 --- a/src/blueapi/service/model.py +++ b/src/blueapi/service/model.py @@ -64,6 +64,11 @@ class TaskRequest(BlueapiBaseModel): """ name: str = Field(description="Name of plan to run") + args: list[Any] = Field( + description="Positional arguments to plan", + default_factory=list, + exclude_if=lambda args: not args, + ) params: Mapping[str, Any] = Field( description="Values for parameters to plan, if any", default_factory=dict ) diff --git a/src/blueapi/worker/task.py b/src/blueapi/worker/task.py index 6dbbaa020..d4cf0e809 100644 --- a/src/blueapi/worker/task.py +++ b/src/blueapi/worker/task.py @@ -1,8 +1,9 @@ import logging from collections.abc import Mapping +from inspect import Parameter, signature from typing import Any, Self, TypedDict -from pydantic import BaseModel, Field, TypeAdapter, ValidationError +from pydantic import Field, TypeAdapter, ValidationError from blueapi.core import BlueskyContext from blueapi.utils import BlueapiBaseModel @@ -16,57 +17,105 @@ class Task(BlueapiBaseModel): """ name: str = Field(description="Name of plan to run") + args: list[Any] = Field( + description="Positional arguments to plan", + default_factory=list, + exclude_if=lambda args: not args, + ) params: Mapping[str, Any] = Field( - description="Values for parameters to plan, if any", default_factory=dict + description="Keyword arguments to plan, if any", + default_factory=dict, ) metadata: dict[str, Any] = Field( description="Any metadata to apply to all runs within this task", default_factory=dict, ) - def prepare_params(self, ctx: BlueskyContext) -> Mapping[str, Any]: + def prepare_params( + self, ctx: BlueskyContext + ) -> tuple[list[Any], Mapping[str, Any]]: + """ + Checks the configured plan parameters against context + + Args: + ctx: Context holding plans and devices + + Returns: + tuple[list[Any], Mapping[str, Any]]: The prepared *args and **kwargs + parameters for the plan. + """ try: - model = _lookup_params(ctx, self) - # Re-create dict manually to avoid nesting in model_dump output - return {field: getattr(model, field) for field in model.__pydantic_fields__} + return self._inner_prepare_params(ctx) except ValidationError as ve: raise TaskParameterError.from_validation_error(ve) from None + except TypeError as te: + raise TaskParameterError( + "Invalid arguments to plan", + [ + InvalidTaskParameter( + loc=(), msg=str(te), type="TypeError", input=None + ) + ], + ) from None + + def _inner_prepare_params( + self, ctx: BlueskyContext + ) -> tuple[list[Any], Mapping[str, Any]]: + plan = ctx.plans[self.name] + func = ctx.plan_functions[self.name] + + sig = signature(func) + bound = sig.bind_partial(*self.args, **self.params) + + # Only validate explicitly supplied arguments. This allows Pydantic's + # default/default_factory to provide injected defaults. + adapter = TypeAdapter(plan.model) + validated = adapter.validate_python(bound.arguments) + + args: list[Any] = [] + kwargs: dict[str, Any] = {} + + for name, parameter in sig.parameters.items(): + supplied = name in bound.arguments + + if supplied: + value = getattr(validated, name) + + match parameter.kind: + case Parameter.POSITIONAL_ONLY: + args.append(value) + case Parameter.VAR_POSITIONAL: + args.extend(value) + case Parameter.POSITIONAL_OR_KEYWORD: + if name in self.params: + kwargs[name] = value + else: + args.append(value) + case Parameter.KEYWORD_ONLY: + kwargs[name] = value + case Parameter.VAR_KEYWORD: + kwargs.update(value) + else: + # Let the generated Pydantic model provide the default. + kwargs[name] = getattr(validated, name) + + return args, kwargs def do_task(self, ctx: BlueskyContext) -> None: LOGGER.info( f"Asked to run plan {self.name} with {self.params} and " f"metadata {self.metadata} for all runs" ) - - func = ctx.plan_functions[self.name] - prepared_params = self.prepare_params(ctx) + plan = ctx.plan_functions[self.name] + prepared_args, prepared_kwargs = self.prepare_params(ctx) ctx.run_engine.md.update(self.metadata) - result = ctx.run_engine(func(**prepared_params)) + result = ctx.run_engine(plan(*prepared_args, **prepared_kwargs)) if isinstance(result, tuple): # pragma: no cover # this is never true if the run_engine is configured correctly return None return result.plan_result -def _lookup_params(ctx: BlueskyContext, task: Task) -> BaseModel: - """ - Checks plan parameters against context - - Args: - ctx: Context holding plans and devices - plan: Plan object including schema - params: Parameter values to be validated against schema - - Returns: - Mapping[str, Any]: _description_ - """ - - plan = ctx.plans[task.name] - model = plan.model - adapter = TypeAdapter(model) - return adapter.validate_python(task.params) - - class InvalidTaskParameter(TypedDict): loc: tuple[str | int, ...] msg: str | None diff --git a/tests/system_tests/test_blueapi_client.py b/tests/system_tests/test_blueapi_client.py index 540efeff4..f16e267cf 100644 --- a/tests/system_tests/test_blueapi_client.py +++ b/tests/system_tests/test_blueapi_client.py @@ -188,16 +188,18 @@ def test_plans_and_devices_are_cached(bc: BlueapiClient): assert bc.devices is bc.devices -def test_plan_argument_errors(bc: BlueapiClient): - det, stage = bc.devices.det, bc.devices.stage - with pytest.raises(TypeError, match="multiple values for detectors"): - bc.plans.count([det], detectors=[stage.x]) - with pytest.raises(TypeError, match="unexpected arguments"): - bc.plans.count([det], foo=1) - with pytest.raises(TypeError, match="too many arguments"): - bc.plans.sleep(1, 2) - with pytest.raises(TypeError, match="Missing argument"): - bc.plans.count() +@pytest.mark.parametrize( + "a,kw,err", + [ + (("det",), {"detectors": ["det"]}, "multiple values for argument 'detectors'"), + (("det",), {"foo": "bar"}, "got an unexpected keyword argument 'foo'"), + ((("det",), 2, 3, 4, 5), {}, "too many positional arguments"), + ((), {}, "Missing value for 'detectors'"), + ], +) +def test_plan_argument_errors(bc: BlueapiClient, a, kw, err: str): + with pytest.raises(TypeError, match=err): + bc.plans.count(*a, **kw) def _record_events(bc: BlueapiClient) -> tuple[list[AnyEvent], int]: diff --git a/tests/system_tests/test_blueapi_system.py b/tests/system_tests/test_blueapi_system.py index 5ee163997..9feda470c 100644 --- a/tests/system_tests/test_blueapi_system.py +++ b/tests/system_tests/test_blueapi_system.py @@ -542,15 +542,26 @@ def test_delete_current_environment(client: BlueapiClient): @pytest.mark.parametrize( "task,user", [ - ( + pytest.param( TaskRequest( name="count", - params={"detectors": ["det"], "num": 3}, + args=[["det"], 3], instrument_session=VALID_INSTRUMENT_SESSION[User.alice], ), User.alice, + id="all_args", ), - ( + pytest.param( + TaskRequest( + name="count", + args=[["det"]], + params={"num": 3}, + instrument_session=VALID_INSTRUMENT_SESSION[User.alice], + ), + User.alice, + id="mixed_args_params", + ), + pytest.param( TaskRequest( name="spec_scan", params={ @@ -577,6 +588,7 @@ def test_delete_current_environment(client: BlueapiClient): instrument_session=VALID_INSTRUMENT_SESSION[User.bob], ), User.bob, + id="all_params", ), ], ) diff --git a/tests/unit_tests/cli/test_cli.py b/tests/unit_tests/cli/test_cli.py index 1840216ff..e8458f0ac 100644 --- a/tests/unit_tests/cli/test_cli.py +++ b/tests/unit_tests/cli/test_cli.py @@ -15,6 +15,7 @@ from bluesky.protocols import Movable from bluesky_stomp.messaging import StompClient from bluesky_stomp.models import MessageTopic +from click import BadParameter from click.testing import CliRunner from ophyd_async.core import AsyncStatus from pydantic import BaseModel @@ -23,7 +24,7 @@ from stomp.connect import StompConnection11 as Connection from blueapi import __version__ -from blueapi.cli.cli import ParametersType, main +from blueapi.cli.cli import ParametersType, PositionalArgType, main from blueapi.cli.format import OutputFormat, fmt_dict from blueapi.client.event_bus import BlueskyStreamingError from blueapi.client.rest import ( @@ -1428,6 +1429,29 @@ def test_task_parameter_type(value, result): assert t.convert(value, None, None) == result +@pytest.mark.parametrize( + "value,result", [([], []), ("[1,2,3]", [1, 2, 3]), (None, None)] +) +def test_positional_arg_type(value, result): + t = PositionalArgType() + assert t.convert(value, None, None) == result + + +@pytest.mark.parametrize( + "value,error", + [ + ("1,2,3", "not valid JSON"), + ("[foo, bar]", "not valid JSON"), + ("23", "should be a list of arguments"), + ('"string"', "should be a list of arguments"), + ], +) +def test_invalid_positional_arg_type(value, error): + t = PositionalArgType() + with pytest.raises(BadParameter, match=error): + assert t.convert(value, None, None) + + @pytest.mark.parametrize( "flag,level", [("--verbose", "DEBUG"), ("--quiet", "ERROR"), ("-v", "DEBUG"), ("-q", "ERROR")], diff --git a/tests/unit_tests/client/test_plan_cache.py b/tests/unit_tests/client/test_plan_cache.py index 3da4d1c9b..6b3624693 100644 --- a/tests/unit_tests/client/test_plan_cache.py +++ b/tests/unit_tests/client/test_plan_cache.py @@ -5,6 +5,7 @@ from blueapi.client.client import PlanCache from blueapi.client.plan_cache import Plan +from blueapi.client.rest import InvalidParametersError, ParameterError from blueapi.service.model import PlanModel, TaskRequest FULL_PLAN = PlanModel( @@ -132,46 +133,43 @@ def test_plan_empty_fallback_help_text(client): @pytest.mark.parametrize( - "args,kwargs,params", + "args,kwargs", [ - p((1,), {}, {"one": 1}, id="required_as_positional"), - p((), {"one": 7}, {"one": 7}, id="required_as_keyword"), - p((1,), {"two": 23}, {"one": 1, "two": 23}, id="all_as_mixed_args_kwargs"), - p((1, 2), {}, {"one": 1, "two": 2}, id="all_as_positional"), - p((), {"one": 21, "two": 42}, {"one": 21, "two": 42}, id="all_as_keyword"), + p((1,), {}, id="required_as_positional"), + p((), {"one": 7}, id="required_as_keyword"), + p((1,), {"two": 23}, id="all_as_mixed_args_kwargs"), + p((1, 2), {}, id="all_as_positional"), + p((), {"one": 21, "two": 42}, id="all_as_keyword"), ], ) -def test_plan_param_mapping(args, kwargs, params): +def test_plan_param_forwarding(args, kwargs): client = Mock() client.instrument_session = "cm12345-1" plan = Plan("foobar", FULL_PLAN, client) plan(*args, **kwargs) client.run_task.assert_called_once_with( - TaskRequest(name="foobar", instrument_session="cm12345-1", params=params) + TaskRequest( + name="foobar", instrument_session="cm12345-1", args=args, params=kwargs + ) ) -@pytest.mark.parametrize( - "args,kwargs,msg", - [ - p((), {}, r"Missing argument\(s\) for \{'one'\}", id="missing_required"), - p((1,), {"one": 7}, "multiple values for one", id="duplicate_required"), - p((1, 2), {"two": 23}, "multiple values for two", id="duplicate_optional"), - p((1, 2, 3), {}, "too many arguments", id="too_many_args"), - p( - (), - {"unknown_key": 42}, - r"got unexpected arguments: \{'unknown_key'\}", - id="unknown_arg", - ), - ], -) -def test_plan_invalid_param_mapping(args, kwargs, msg): +def test_plan_invalid_param_mapping(): client = Mock() client.instrument_session = "cm12345-1" + client.run_task.side_effect = InvalidParametersError( + [ + ParameterError( + msg="foo is not valid", + loc=["body", "params", "foo"], + input="bar", + type="invalid", + ) + ] + ) plan = Plan("foobar", FULL_PLAN, client) - with pytest.raises(TypeError, match=msg): - plan(*args, **kwargs) - client.run_task.assert_not_called() + with pytest.raises(TypeError, match="foo is not valid"): + plan(1, 2, foo="bar") + client.run_task.assert_called_once() diff --git a/tests/unit_tests/client/test_rest.py b/tests/unit_tests/client/test_rest.py index 8c8c004cd..b756b0b75 100644 --- a/tests/unit_tests/client/test_rest.py +++ b/tests/unit_tests/client/test_rest.py @@ -281,6 +281,16 @@ def test_parameter_error_extra_string(): assert str(p1) == "Unexpected field 'foo'" +def test_parameter_error_unknown_general_error(): + p1 = ParameterError( + loc=["body", "params"], + msg="Something else is wrong", + type="TypeError", + input=None, + ) + assert str(p1) == "Something else is wrong" + + def test_parameter_error_other_string(): p1 = ParameterError( loc=["body", "parameters", "field_one", 0], diff --git a/tests/unit_tests/service/test_interface.py b/tests/unit_tests/service/test_interface.py index ef44a558d..db4e68760 100644 --- a/tests/unit_tests/service/test_interface.py +++ b/tests/unit_tests/service/test_interface.py @@ -195,12 +195,15 @@ def test_get_device(context_mock: MagicMock): assert interface.get_device("non_existing_device") +@pytest.mark.parametrize("args,kwargs", (([], {"repeats": 42}), ([42], {}))) @patch("blueapi.service.interface.context") -def test_submit_task(context_mock: MagicMock): +def test_submit_task(context_mock: MagicMock, args, kwargs): context = BlueskyContext() - context.register_plan(my_plan) + context.register_plan(my_second_plan) task = TaskRequest( - name="my_plan", + name="my_second_plan", + args=args, + params=kwargs, instrument_session=FAKE_INSTRUMENT_SESSION, ) context_mock.return_value = context diff --git a/tests/unit_tests/service/test_rest_api.py b/tests/unit_tests/service/test_rest_api.py index a02b1e301..115ae5aae 100644 --- a/tests/unit_tests/service/test_rest_api.py +++ b/tests/unit_tests/service/test_rest_api.py @@ -371,6 +371,31 @@ def test_create_task_parameter_error(mock_runner: Mock, client: TestClient) -> N } +def test_create_task_parameter_type_error( + mock_runner: Mock, client: TestClient +) -> None: + mock_runner.run.side_effect = [TypeError("Some other error")] + + response = client.post( + "/tasks", + json={ + "name": "my-plan", + "instrument_session": FAKE_INSTRUMENT_SESSION, + }, + ) + assert response.status_code == 422 + assert response.json() == { + "detail": [ + { + "input": None, + "loc": ["body", "params"], + "msg": "Some other error", + "type": "invalid", + } + ] + } + + def test_put_plan_begins_task(client: TestClient) -> None: task_id = "04cd9aa6-b902-414b-ae4b-49ea4200e957" diff --git a/tests/unit_tests/worker/test_task_worker.py b/tests/unit_tests/worker/test_task_worker.py index f6fe50c0d..8f0a1269c 100644 --- a/tests/unit_tests/worker/test_task_worker.py +++ b/tests/unit_tests/worker/test_task_worker.py @@ -34,6 +34,7 @@ WorkerState, ) from blueapi.worker.event import TaskResult, TaskStatusEnum +from blueapi.worker.task import TaskParameterError _SIMPLE_TASK = Task(name="sleep", params={"time": 0.0}) _LONG_TASK = Task(name="sleep", params={"time": 1.0}) @@ -724,7 +725,7 @@ def injected_device_plan( yield from () context.register_plan(injected_device_plan) - params = Task(name="injected_device_plan").prepare_params(context) + _, params = Task(name="injected_device_plan").prepare_params(context) assert params["dev"] == fake_device @@ -840,7 +841,7 @@ def test_injected_composite_devices_are_found( context: BlueskyContext, ): context.register_plan(injected_device_plan) - params = Task(name="injected_device_plan").prepare_params(context) + _, params = Task(name="injected_device_plan").prepare_params(context) assert params["composite"].fake_device == fake_device assert params["composite"].second_fake_device == second_fake_device @@ -861,7 +862,7 @@ def test_injected_composite_with_pydantic_dataclass( second_fake_device: FakeDevice, ): context.register_plan(injected_dataclass_device_plan) - params = Task(name="injected_dataclass_device_plan").prepare_params(context) + _, params = Task(name="injected_dataclass_device_plan").prepare_params(context) assert params["composite"].fake_device == fake_device assert params["composite"].second_fake_device == second_fake_device @@ -872,7 +873,7 @@ def test_injected_composite_with_standard_dataclass( second_fake_device: FakeDevice, ): context.register_plan(injected_standard_dataclass_device_plan) - params = Task(name="injected_standard_dataclass_device_plan").prepare_params( + _, params = Task(name="injected_standard_dataclass_device_plan").prepare_params( context ) assert params["composite"].fake_device == fake_device @@ -887,7 +888,9 @@ def test_plan_module_with_composite_devices_can_be_loaded_before_device_module( context_without_devices.register_plan(injected_device_plan) context_without_devices.register_device(fake_device) context_without_devices.register_device(second_fake_device) - params = Task(name="injected_device_plan").prepare_params(context_without_devices) + _, params = Task(name="injected_device_plan").prepare_params( + context_without_devices + ) assert params["composite"].fake_device == fake_device assert params["composite"].second_fake_device == second_fake_device @@ -949,3 +952,178 @@ def test_worker_event_task_id(): def test_worker_event_no_task_id(): event = WorkerEvent(state=WorkerState.IDLE, task_status=None) assert event.task_id is None + + +def test_task_worker_passes_positional_args( + context: BlueskyContext, +) -> None: + def positional_plan(value: int) -> MsgGenerator: + yield from () + + context.register_plan(positional_plan) + + task = Task(name="positional_plan", args=[42]) + args, kwargs = task.prepare_params(context) + + assert args == [42] + assert kwargs == {} + + +def test_task_worker_passes_multiple_positional_args( + context: BlueskyContext, +) -> None: + def positional_plan(first: int, second: int) -> MsgGenerator: + yield from () + + context.register_plan(positional_plan) + task = Task(name="positional_plan", args=[1, 2]) + args, kwargs = task.prepare_params(context) + + assert args == [1, 2] + assert kwargs == {} + + +def test_task_worker_passes_positional_and_keyword_args( + context: BlueskyContext, +) -> None: + def mixed_plan( + first: int, + second: int, + *, + third: int, + ) -> MsgGenerator: + yield from () + + context.register_plan(mixed_plan) + task = Task( + name="mixed_plan", + args=[1, 2], + params={"third": 3}, + ) + args, kwargs = task.prepare_params(context) + assert args == [1, 2] + assert kwargs == {"third": 3} + + +def test_task_worker_resolves_positional_device( + context: BlueskyContext, + fake_device: FakeDevice, +) -> None: + def positional_device_plan(device: FakeDevice) -> MsgGenerator: + yield from () + + context.register_plan(positional_device_plan) + + task = Task(name="positional_device_plan", args=[fake_device.name]) + args, kwargs = task.prepare_params(context) + assert args == [fake_device] + assert kwargs == {} + + +def test_positional_only_plan(fake_device: FakeDevice) -> None: + def positional_only_plan(val: int, /, device: FakeDevice) -> MsgGenerator: + yield from () + + context = BlueskyContext() + context.register_device(fake_device) + context.register_plan(positional_only_plan) + + task = Task( + name="positional_only_plan", + args=[1], + params={"device": fake_device.name}, + ) + + args, kwargs = task.prepare_params(context) + assert args == [1] + assert kwargs == {"device": fake_device} + + +def test_var_positional_plan(fake_device: FakeDevice) -> None: + def positional_device_plan(val: int, *device: FakeDevice) -> MsgGenerator: + yield from () + + context = BlueskyContext() + context.register_device(fake_device) + context.register_plan(positional_device_plan) + + task = Task( + name="positional_device_plan", + args=[1, fake_device.name, fake_device.name], + ) + args, kwargs = task.prepare_params(context) + assert args == [1, fake_device, fake_device] + assert kwargs == {} + + +def test_positional_only_and_var_positional_plan( + fake_device: FakeDevice, +) -> None: + def positional_device_plan(val: int, /, *device: FakeDevice) -> MsgGenerator: + yield from () + + context = BlueskyContext() + context.register_device(fake_device) + context.register_plan(positional_device_plan) + + task = Task( + name="positional_device_plan", + args=[1, fake_device.name, fake_device.name], + ) + + args, kwargs = task.prepare_params(context) + assert args == [1, fake_device, fake_device] + assert kwargs == {} + + +def test_var_keyword_plan(fake_device: FakeDevice) -> None: + def keyword_device_plan(val: int, **devices: FakeDevice) -> MsgGenerator: + yield from () + + context = BlueskyContext() + context.register_device(fake_device) + context.register_plan(keyword_device_plan) + + task = Task( + name="keyword_device_plan", + args=[1], + params={"first": fake_device.name, "second": fake_device.name}, + ) + args, kwargs = task.prepare_params(context) + assert args == [1] + assert kwargs == {"first": fake_device, "second": fake_device} + + +def test_positional_only_and_var_keyword_plan( + fake_device: FakeDevice, +) -> None: + def positional_only_plan(val: int, /, **devices: FakeDevice) -> MsgGenerator: + yield from () + + context = BlueskyContext() + context.register_device(fake_device) + context.register_plan(positional_only_plan) + + task = Task( + name="positional_only_plan", + args=[1], + params={"first": fake_device.name, "second": fake_device.name}, + ) + args, kwargs = task.prepare_params(context) + assert args == [1] + assert kwargs == {"first": fake_device, "second": fake_device} + + +def test_arg_in_args_and_kwargs(): + + def positionals(one: int, two: int) -> MsgGenerator: + yield from [] + + context = BlueskyContext() + context.register_plan(positionals) + + task = Task(name="positionals", args=[1], params={"one": 2}) + with pytest.raises(TaskParameterError, match="Invalid arguments to plan") as tpe: + task.prepare_params(context) + + assert tpe.value.errors[0]["msg"] == "multiple values for argument 'one'"