From 5e62fb3a6bdaaa4a73631bcafd845f190a56c713 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Thu, 20 Apr 2023 14:26:37 +0000 Subject: [PATCH 01/27] simple fastapi setup --- .vscode/launch.json | 13 +++++++- pyproject.toml | 2 ++ src/blueapi/__init__.py | 8 ++++- src/blueapi/rest/__init__.py | 0 src/blueapi/rest/app.py | 58 +++++++++++++++++++++++++++++++++++ src/blueapi/service/app.py | 56 +++++++++++++++++++++++++++++++-- src/blueapi/service/rest.py | 7 +++++ src/blueapi/service/routes.py | 38 +++++++++++++++++++++++ 8 files changed, 177 insertions(+), 5 deletions(-) create mode 100644 src/blueapi/rest/__init__.py create mode 100644 src/blueapi/rest/app.py create mode 100644 src/blueapi/service/rest.py create mode 100644 src/blueapi/service/routes.py diff --git a/.vscode/launch.json b/.vscode/launch.json index e6fa806cb..7fe92cbf6 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -4,6 +4,17 @@ // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 "version": "0.2.0", "configurations": [ + { + "name": "Python: FastAPI", + "type": "python", + "request": "launch", + "module": "uvicorn", + "args": [ + "src.blueapi.service.rest:app" + ], + "jinja": true, + "justMyCode": true + }, { "name": "Debug Unit Test", "type": "python", @@ -68,4 +79,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/pyproject.toml b/pyproject.toml index ae12cd371..8e654b7b7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,6 +22,8 @@ dependencies = [ "scanspec", "PyYAML", "click", + "fastapi", + "uvicorn", ] dynamic = ["version"] license.file = "LICENSE" diff --git a/src/blueapi/__init__.py b/src/blueapi/__init__.py index 6cbf62716..ffb0055a1 100644 --- a/src/blueapi/__init__.py +++ b/src/blueapi/__init__.py @@ -1,6 +1,12 @@ from importlib.metadata import version +from blueapi.core.context import BlueskyContext + +from blueapi.worker.reworker import RunEngineWorker __version__ = version("blueapi") del version -__all__ = ["__version__"] +context = BlueskyContext() +worker = RunEngineWorker(context) + +__all__ = ["__version__", "context", "worker"] diff --git a/src/blueapi/rest/__init__.py b/src/blueapi/rest/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/blueapi/rest/app.py b/src/blueapi/rest/app.py new file mode 100644 index 000000000..f0026d96e --- /dev/null +++ b/src/blueapi/rest/app.py @@ -0,0 +1,58 @@ +from pathlib import Path +from typing import Mapping, Optional +from fastapi import FastAPI +from blueapi.core.context import BlueskyContext +from blueapi.core.event import EventStream +from blueapi.messaging.stomptemplate import StompMessagingTemplate, MessagingTemplate +from blueapi.utils.config import ConfigLoader +from blueapi.worker import run_worker_in_own_thread +from blueapi.worker.reworker import RunEngineWorker +from blueapi.config import ApplicationConfig +from blueapi.worker.worker import Worker +import logging + +app = () + + +class RestApi: + _config: ApplicationConfig + _message_bus: MessagingTemplate + _ctx: BlueskyContext + _worker: Worker + _app: FastAPI + + def __init__(self, config: ApplicationConfig) -> None: + self._config = config + self._ctx = BlueskyContext() + self._ctx.with_startup_script(self._config.env.startup_script) + self._worker = RunEngineWorker(self._ctx) + self._worker_future = run_worker_in_own_thread(self._worker) + self._message_bus = StompMessagingTemplate.autoconfigured(config.stomp) + + def run(self) -> None: + logging.basicConfig(level=self._config.logging.level) + + self._worker.data_events.subscribe( + lambda event, corr_id: self._message_bus.send( + "public.worker.event.data", event, None, corr_id + ) + ) + self._worker.progress_events.subscribe( + lambda event, corr_id: self._message_bus.send( + "public.worker.event.progress", event, None, corr_id + ) + ) + + self._message_bus.connect() + self._app = FastAPI() + + self._worker.run_forever() + + +def start(config_path: Optional[Path] = None): + loader = ConfigLoader(ApplicationConfig) + if config_path is not None: + loader.use_yaml_or_json_file(config_path) + config = loader.load() + + RestApi(config).run() diff --git a/src/blueapi/service/app.py b/src/blueapi/service/app.py index d6fc1a71c..bf3fa5dcc 100644 --- a/src/blueapi/service/app.py +++ b/src/blueapi/service/app.py @@ -2,10 +2,16 @@ import uuid from typing import Mapping +from fastapi import FastAPI + from blueapi.config import ApplicationConfig from blueapi.core import BlueskyContext, EventStream from blueapi.messaging import MessageContext, MessagingTemplate, StompMessagingTemplate from blueapi.worker import RunEngineWorker, RunPlan, Worker +from blueapi import context, worker +from blueapi.worker.multithread import run_worker_in_own_thread + +from .routes import router from .model import ( DeviceModel, @@ -48,7 +54,7 @@ def run(self) -> None: } ) - self._template.subscribe("worker.run", self._on_run_request) + self._template.subscribe(" ", self._on_run_request) self._template.subscribe("worker.plans", self._get_plans) self._template.subscribe("worker.devices", self._get_devices) @@ -95,5 +101,49 @@ def _get_devices( self._template.send(message_context.reply_destination, response) -def start(config: ApplicationConfig): - Service(config).run() +##need to globally, start the worker and message bus. +## message bus needs a config file, +## worker needs a context, +## context needs a config file. + +## so how about, we set up a context somewhere (in context module), +## we start up the worker with the context, +# THEN in this start we load config into the context and load the message bus from the config. + +## the rest api never needs to interact with the message bus anyways... it only interacts with context or worker. + + +def start(config_path: Optional[Path] = None): + # 1. load config and setup logging + loader = ConfigLoader(ApplicationConfig) + if config_path is not None: + loader.use_yaml_or_json_file(config_path) + config = loader.load() + logging.basicConfig(level=config.logging.level) + + # 2. set context with startup script + context.with_startup_script(config.env.startup_script) + + # 3. run the worker in it's own thread + worker_future = run_worker_in_own_thread(worker) + + # 4. create a message bus and subscribe all relevant worker docs to it + message_bus = StompMessagingTemplate.autoconfigured(config.stomp) + worker.data_events.subscribe( + lambda event, corr_id: message_bus.send( + "public.worker.event.data", event, None, corr_id + ) + ) + worker.progress_events.subscribe( + lambda event, corr_id: message_bus.send( + "public.worker.event.progress", event, None, corr_id + ) + ) + + # 5. start the message bus + message_bus.connect() + + # 7. run the worker forever + worker.run_forever() + + # Service(config).run() diff --git a/src/blueapi/service/rest.py b/src/blueapi/service/rest.py new file mode 100644 index 000000000..905e0e8f3 --- /dev/null +++ b/src/blueapi/service/rest.py @@ -0,0 +1,7 @@ +from blueapi.service.routes import router +from fastapi import FastAPI + +app = FastAPI() + +# here, do app.include_router from all the other routes you want. +app.include_router(router) diff --git a/src/blueapi/service/routes.py b/src/blueapi/service/routes.py new file mode 100644 index 000000000..16e41aacc --- /dev/null +++ b/src/blueapi/service/routes.py @@ -0,0 +1,38 @@ +from fastapi import APIRouter +from blueapi import context, worker + +router = APIRouter() + + +@router.get("/plans") +async def get_plans(): + context.plans + ... + + +@router.get("/plan/{name}") +async def get_plan_by_name(name: str): + try: + context.plans[name] + except IndexError: + raise Exception() # really, return a 404. + + +@router.get("/devices") +async def get_devices(): + context.devices + + +@router.get("/device/{name}") +async def get_device_by_name(name: str): + try: + context.plans[name] + except IndexError: + raise Exception() # really, return a 404. + + +@router.put("task/{name}") +async def execute_task(name: str): + ##basically in here, do the same thing the service once did... + # worker.submit_task(name, task) + pass From 2c56053af9623b6003fe4259f6789f9914c58fe1 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Fri, 5 May 2023 12:51:37 +0000 Subject: [PATCH 02/27] squashing commits; rebased onto master and modified some vscode settings. WIP. --- .devcontainer/Dockerfile | 2 +- .vscode/launch.json | 3 +- .vscode/settings.json | 4 +- pyproject.toml | 2 + src/blueapi/__init__.py | 7 +- src/blueapi/cli/cli.py | 69 ++++++++------- src/blueapi/rest/__init__.py | 0 src/blueapi/rest/app.py | 58 ------------- src/blueapi/service/__init__.py | 3 +- src/blueapi/service/app.py | 149 -------------------------------- src/blueapi/service/handler.py | 73 ++++++++++++++++ src/blueapi/service/main.py | 56 ++++++++++++ src/blueapi/service/rest.py | 7 -- src/blueapi/service/routes.py | 38 -------- tests/service/test_rest_api.py | 139 +++++++++++++++++++++++++++++ 15 files changed, 313 insertions(+), 297 deletions(-) delete mode 100644 src/blueapi/rest/__init__.py delete mode 100644 src/blueapi/rest/app.py delete mode 100644 src/blueapi/service/app.py create mode 100644 src/blueapi/service/handler.py create mode 100644 src/blueapi/service/main.py delete mode 100644 src/blueapi/service/rest.py delete mode 100644 src/blueapi/service/routes.py create mode 100644 tests/service/test_rest_api.py diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index de19c276e..b669993ac 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -34,4 +34,4 @@ ENV PATH=/venv/bin:$PATH # change this entrypoint if it is not the same as the repo ENTRYPOINT ["blueapi"] -CMD ["worker"] +CMD ["run"] diff --git a/.vscode/launch.json b/.vscode/launch.json index 7fe92cbf6..2fb4dc60b 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -10,7 +10,8 @@ "request": "launch", "module": "uvicorn", "args": [ - "src.blueapi.service.rest:app" + "src.blueapi.rest.main:app", + "--reload" ], "jinja": true, "justMyCode": true diff --git a/.vscode/settings.json b/.vscode/settings.json index 3f37e62ff..7eac220fe 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -13,5 +13,5 @@ "source.organizeImports": true }, "esbonio.server.enabled": true, - "esbonio.sphinx.confDir": "" -} \ No newline at end of file + "esbonio.sphinx.confDir": "", +} diff --git a/pyproject.toml b/pyproject.toml index 8e654b7b7..c90b0e7df 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,6 +24,7 @@ dependencies = [ "click", "fastapi", "uvicorn", + "httpx", ] dynamic = ["version"] license.file = "LICENSE" @@ -46,6 +47,7 @@ dev = [ "tox-direct", "types-mock", "types-PyYAML", + "types-requests", ] [project.scripts] diff --git a/src/blueapi/__init__.py b/src/blueapi/__init__.py index ffb0055a1..bdccda11d 100644 --- a/src/blueapi/__init__.py +++ b/src/blueapi/__init__.py @@ -1,12 +1,7 @@ from importlib.metadata import version -from blueapi.core.context import BlueskyContext - -from blueapi.worker.reworker import RunEngineWorker __version__ = version("blueapi") del version -context = BlueskyContext() -worker = RunEngineWorker(context) -__all__ = ["__version__", "context", "worker"] +__all__ = ["__version__"] diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 53f60bfbd..d7997b491 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -1,16 +1,18 @@ import json import logging -from pathlib import Path -from typing import Optional +from pprint import pprint import click +import requests +from requests.exceptions import ConnectionError from blueapi import __version__ from blueapi.config import ApplicationConfig, ConfigLoader from blueapi.messaging import StompMessagingTemplate +from blueapi.service.main import app -from .amq import AmqClient -from .updates import CliEventRenderer +from pathlib import Path +from typing import Optional @click.group(invoke_without_command=True) @@ -30,13 +32,14 @@ def main(ctx, config: Optional[Path]) -> None: print("Please invoke subcommand!") -@main.command(name="worker") +@click.version_option(version=__version__) +@main.command(name="run") @click.pass_obj -def start_worker(obj: dict) -> None: - from blueapi.service import start +def start_application(obj: dict): + import uvicorn config: ApplicationConfig = obj["config"] - start(config) + uvicorn.run(app, host=settings.host, port=int(settings.port)) @main.group() @@ -49,39 +52,39 @@ def controller(ctx) -> None: ctx.ensure_object(dict) config: ApplicationConfig = ctx.obj["config"] logging.basicConfig(level=config.logging.level) - client = AmqClient(StompMessagingTemplate.autoconfigured(config.stomp)) - ctx.obj["client"] = client - client.app.connect() + + +def check_connection(func): + def wrapper(*args, **kwargs): + try: + func(*args, **kwargs) + except ConnectionError: + print("Failed to establish connection.") + + return wrapper @controller.command(name="plans") -@click.pass_context -def get_plans(ctx) -> None: - client: AmqClient = ctx.obj["client"] - plans = client.get_plans() - print("PLANS") - for plan in plans.plans: - print("\t" + plan.name) +@check_connection +def get_plans() -> None: + resp = requests.get(f"{settings.url}/plans") + print(f"Response returned with {resp.status_code}: ") + pprint(resp.json()) @controller.command(name="devices") -@click.pass_context -def get_devices(ctx) -> None: - client: AmqClient = ctx.obj["client"] - print(client.get_devices().devices) +@check_connection +def get_devices() -> None: + resp = requests.get(f"{settings.url}/devices") + print(f"Response returned with {resp.status_code}: ") + pprint(resp.json()) @controller.command(name="run") @click.argument("name", type=str) @click.option("-p", "--parameters", type=str, help="Parameters as valid JSON") -@click.pass_context -def run_plan(ctx, name: str, parameters: str) -> None: - client: AmqClient = ctx.obj["client"] - renderer = CliEventRenderer() - client.run_plan( - name, - json.loads(parameters), - renderer.on_worker_event, - renderer.on_progress_event, - timeout=120.0, - ) +@check_connection +def run_plan(name: str, parameters: str) -> None: + resp = requests.put(f"{settings.url}/task/{name}", json=json.loads(parameters)) + print(f"Response returned with {resp.status_code}: ") + pprint(resp.json()) diff --git a/src/blueapi/rest/__init__.py b/src/blueapi/rest/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/src/blueapi/rest/app.py b/src/blueapi/rest/app.py deleted file mode 100644 index f0026d96e..000000000 --- a/src/blueapi/rest/app.py +++ /dev/null @@ -1,58 +0,0 @@ -from pathlib import Path -from typing import Mapping, Optional -from fastapi import FastAPI -from blueapi.core.context import BlueskyContext -from blueapi.core.event import EventStream -from blueapi.messaging.stomptemplate import StompMessagingTemplate, MessagingTemplate -from blueapi.utils.config import ConfigLoader -from blueapi.worker import run_worker_in_own_thread -from blueapi.worker.reworker import RunEngineWorker -from blueapi.config import ApplicationConfig -from blueapi.worker.worker import Worker -import logging - -app = () - - -class RestApi: - _config: ApplicationConfig - _message_bus: MessagingTemplate - _ctx: BlueskyContext - _worker: Worker - _app: FastAPI - - def __init__(self, config: ApplicationConfig) -> None: - self._config = config - self._ctx = BlueskyContext() - self._ctx.with_startup_script(self._config.env.startup_script) - self._worker = RunEngineWorker(self._ctx) - self._worker_future = run_worker_in_own_thread(self._worker) - self._message_bus = StompMessagingTemplate.autoconfigured(config.stomp) - - def run(self) -> None: - logging.basicConfig(level=self._config.logging.level) - - self._worker.data_events.subscribe( - lambda event, corr_id: self._message_bus.send( - "public.worker.event.data", event, None, corr_id - ) - ) - self._worker.progress_events.subscribe( - lambda event, corr_id: self._message_bus.send( - "public.worker.event.progress", event, None, corr_id - ) - ) - - self._message_bus.connect() - self._app = FastAPI() - - self._worker.run_forever() - - -def start(config_path: Optional[Path] = None): - loader = ConfigLoader(ApplicationConfig) - if config_path is not None: - loader.use_yaml_or_json_file(config_path) - config = loader.load() - - RestApi(config).run() diff --git a/src/blueapi/service/__init__.py b/src/blueapi/service/__init__.py index 95704ed7c..7c2fa404c 100644 --- a/src/blueapi/service/__init__.py +++ b/src/blueapi/service/__init__.py @@ -1,4 +1,3 @@ -from .app import start from .model import DeviceModel, PlanModel -__all__ = ["start", "PlanModel", "DeviceModel"] +__all__ = ["PlanModel", "DeviceModel"] diff --git a/src/blueapi/service/app.py b/src/blueapi/service/app.py deleted file mode 100644 index bf3fa5dcc..000000000 --- a/src/blueapi/service/app.py +++ /dev/null @@ -1,149 +0,0 @@ -import logging -import uuid -from typing import Mapping - -from fastapi import FastAPI - -from blueapi.config import ApplicationConfig -from blueapi.core import BlueskyContext, EventStream -from blueapi.messaging import MessageContext, MessagingTemplate, StompMessagingTemplate -from blueapi.worker import RunEngineWorker, RunPlan, Worker -from blueapi import context, worker -from blueapi.worker.multithread import run_worker_in_own_thread - -from .routes import router - -from .model import ( - DeviceModel, - DeviceRequest, - DeviceResponse, - PlanModel, - PlanRequest, - PlanResponse, - TaskResponse, -) - - -class Service: - _config: ApplicationConfig - _ctx: BlueskyContext - _worker: Worker - _template: MessagingTemplate - - def __init__(self, config: ApplicationConfig) -> None: - self._config = config - self._ctx = BlueskyContext() - self._ctx.with_startup_script(self._config.env.startup_script) - self._worker = RunEngineWorker(self._ctx) - self._template = StompMessagingTemplate.autoconfigured(config.stomp) - - def run(self) -> None: - logging.basicConfig(level=self._config.logging.level) - - self._publish_event_streams( - { - self._worker.worker_events: self._template.destinations.topic( - "public.worker.event" - ), - self._worker.progress_events: self._template.destinations.topic( - "public.worker.event.progress" - ), - self._worker.data_events: self._template.destinations.topic( - "public.worker.event.data" - ), - } - ) - - self._template.subscribe(" ", self._on_run_request) - self._template.subscribe("worker.plans", self._get_plans) - self._template.subscribe("worker.devices", self._get_devices) - - self._template.connect() - - self._worker.run() - - def _publish_event_streams( - self, streams_to_destinations: Mapping[EventStream, str] - ) -> None: - for stream, destination in streams_to_destinations.items(): - self._publish_event_stream(stream, destination) - - def _publish_event_stream(self, stream: EventStream, destination: str) -> None: - stream.subscribe( - lambda event, correlation_id: self._template.send( - destination, event, None, correlation_id - ) - ) - - def _on_run_request(self, message_context: MessageContext, task: RunPlan) -> None: - correlation_id = message_context.correlation_id or str(uuid.uuid1()) - self._worker.submit_task(correlation_id, task) - - reply_queue = message_context.reply_destination - if reply_queue is not None: - response = TaskResponse(task_name=correlation_id) - self._template.send(reply_queue, response) - - def _get_plans(self, message_context: MessageContext, message: PlanRequest) -> None: - plans = list(map(PlanModel.from_plan, self._ctx.plans.values())) - response = PlanResponse(plans=plans) - - assert message_context.reply_destination is not None - self._template.send(message_context.reply_destination, response) - - def _get_devices( - self, message_context: MessageContext, message: DeviceRequest - ) -> None: - devices = list(map(DeviceModel.from_device, self._ctx.devices.values())) - response = DeviceResponse(devices=devices) - - assert message_context.reply_destination is not None - self._template.send(message_context.reply_destination, response) - - -##need to globally, start the worker and message bus. -## message bus needs a config file, -## worker needs a context, -## context needs a config file. - -## so how about, we set up a context somewhere (in context module), -## we start up the worker with the context, -# THEN in this start we load config into the context and load the message bus from the config. - -## the rest api never needs to interact with the message bus anyways... it only interacts with context or worker. - - -def start(config_path: Optional[Path] = None): - # 1. load config and setup logging - loader = ConfigLoader(ApplicationConfig) - if config_path is not None: - loader.use_yaml_or_json_file(config_path) - config = loader.load() - logging.basicConfig(level=config.logging.level) - - # 2. set context with startup script - context.with_startup_script(config.env.startup_script) - - # 3. run the worker in it's own thread - worker_future = run_worker_in_own_thread(worker) - - # 4. create a message bus and subscribe all relevant worker docs to it - message_bus = StompMessagingTemplate.autoconfigured(config.stomp) - worker.data_events.subscribe( - lambda event, corr_id: message_bus.send( - "public.worker.event.data", event, None, corr_id - ) - ) - worker.progress_events.subscribe( - lambda event, corr_id: message_bus.send( - "public.worker.event.progress", event, None, corr_id - ) - ) - - # 5. start the message bus - message_bus.connect() - - # 7. run the worker forever - worker.run_forever() - - # Service(config).run() diff --git a/src/blueapi/service/handler.py b/src/blueapi/service/handler.py new file mode 100644 index 000000000..cd080da83 --- /dev/null +++ b/src/blueapi/service/handler.py @@ -0,0 +1,73 @@ +import logging +from functools import lru_cache +from pathlib import Path +from typing import Optional + +from blueapi.config import ApplicationConfig, AppSettings +from blueapi.core import BlueskyContext +from blueapi.messaging import StompMessagingTemplate +from blueapi.messaging.base import MessagingTemplate +from blueapi.utils import ConfigLoader +from blueapi.worker.reworker import RunEngineWorker +from blueapi.worker.worker import Worker + +settings = AppSettings() + + +class Handler: + context: BlueskyContext + worker: Worker + config: ApplicationConfig + message_bus: MessagingTemplate + + def __init__(self, config_path: Optional[Path]) -> None: + self.context = BlueskyContext() + + loader = ConfigLoader(ApplicationConfig) + if config_path is not None: + loader.use_yaml_or_json_file(config_path) + self.config = loader.load() + logging.basicConfig(level=self.config.logging.level) + + self.context.with_startup_script(self.config.env.startup_script) + + self.worker = RunEngineWorker(self.context) + self.message_bus = StompMessagingTemplate.autoconfigured(self.config.stomp) + + def start(self) -> None: + self.worker.start() + + self.worker.data_events.subscribe( + lambda event, corr_id: self.message_bus.send( + "public.worker.event.data", event, None, corr_id + ) + ) + self.worker.progress_events.subscribe( + lambda event, corr_id: self.message_bus.send( + "public.worker.event.progress", event, None, corr_id + ) + ) + + self.message_bus.connect() + + def stop(self) -> None: + self.worker.stop() + self.message_bus.disconnect() + + +@lru_cache(maxsize=50) +def get_handler() -> Handler: + """Retrieve the handler which wraps the bluesky context, worker and message bus.""" + config_path: Optional[Path] = ( + Path(settings.app_config_path) if settings.app_config_path is not None else None + ) + + handler = Handler(config_path) + handler.start() + return handler + + +def teardown_handler() -> None: + """Stop all handler tasks. Does nothing if setup_handler has not been called.""" + handler = get_handler() + handler.stop() diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py new file mode 100644 index 000000000..01fd8b9ec --- /dev/null +++ b/src/blueapi/service/main.py @@ -0,0 +1,56 @@ +from fastapi import Body, Depends, FastAPI, HTTPException + +from blueapi.worker import RunPlan + +from .handler import Handler, get_handler, teardown_handler +from .model import DeviceModel, DeviceResponse, PlanModel, PlanResponse + +app = FastAPI( + docs_url="/docs", on_shutdown=[teardown_handler], on_startup=[get_handler] +) + + +@app.get("/plans", response_model=PlanResponse) +async def get_plans(handler: Handler = Depends(get_handler)): + return PlanResponse( + plans=[PlanModel.from_plan(plan) for plan in handler.context.plans.values()] + ) + + +@app.get("/plan/{name}", response_model=PlanModel) +async def get_plan_by_name(name: str, handler: Handler = Depends(get_handler)): + try: + return PlanModel.from_plan(handler.context.plans[name]) + except KeyError: + raise HTTPException(status_code=404, detail="Item not found") + + +@app.get("/devices", response_model=DeviceResponse) +async def get_devices(handler: Handler = Depends(get_handler)): + return DeviceResponse( + devices=[ + DeviceModel.from_device(device) + for device in handler.context.devices.values() + ] + ) + + +@app.get("/device/{name}", response_model=DeviceModel) +async def get_device_by_name(name: str, handler: Handler = Depends(get_handler)): + try: + return DeviceModel.from_device(handler.context.devices[name]) + except KeyError: + raise HTTPException(status_code=404, detail="Item not found") + + +@app.put("/task/{name}") +async def execute_task( + name: str, + task: RunPlan = Body( + ..., example={"name": "count", "params": {"detectors": ["x"]}} + ), + handler: Handler = Depends(get_handler), +): + # basically in here, do the same thing the service once did... + handler.worker.submit_task(name, task) + pass diff --git a/src/blueapi/service/rest.py b/src/blueapi/service/rest.py deleted file mode 100644 index 905e0e8f3..000000000 --- a/src/blueapi/service/rest.py +++ /dev/null @@ -1,7 +0,0 @@ -from blueapi.service.routes import router -from fastapi import FastAPI - -app = FastAPI() - -# here, do app.include_router from all the other routes you want. -app.include_router(router) diff --git a/src/blueapi/service/routes.py b/src/blueapi/service/routes.py deleted file mode 100644 index 16e41aacc..000000000 --- a/src/blueapi/service/routes.py +++ /dev/null @@ -1,38 +0,0 @@ -from fastapi import APIRouter -from blueapi import context, worker - -router = APIRouter() - - -@router.get("/plans") -async def get_plans(): - context.plans - ... - - -@router.get("/plan/{name}") -async def get_plan_by_name(name: str): - try: - context.plans[name] - except IndexError: - raise Exception() # really, return a 404. - - -@router.get("/devices") -async def get_devices(): - context.devices - - -@router.get("/device/{name}") -async def get_device_by_name(name: str): - try: - context.plans[name] - except IndexError: - raise Exception() # really, return a 404. - - -@router.put("task/{name}") -async def execute_task(name: str): - ##basically in here, do the same thing the service once did... - # worker.submit_task(name, task) - pass diff --git a/tests/service/test_rest_api.py b/tests/service/test_rest_api.py new file mode 100644 index 000000000..08d5cf86a --- /dev/null +++ b/tests/service/test_rest_api.py @@ -0,0 +1,139 @@ +from ast import literal_eval +from dataclasses import dataclass + +from fastapi.testclient import TestClient +from pydantic import BaseModel + +from blueapi.core.bluesky_types import Plan +from blueapi.core.context import BlueskyContext +from blueapi.service.handler import get_handler +from blueapi.service.main import app +from blueapi.worker import RunEngineWorker +from blueapi.worker.task import ActiveTask + +# client = TestClient(app) + + +class MockHandler: + context: BlueskyContext + worker: RunEngineWorker + + def __init__(self) -> None: + self.context = BlueskyContext() + self.worker = RunEngineWorker(self.context) + + +class Client: + def __init__(self, handler: MockHandler) -> None: + """Create tester object""" + self.handler = handler + + @property + def client(self) -> TestClient: + app.dependency_overrides[get_handler] = lambda: self.handler + return TestClient(app) + + +def test_get_plans() -> None: + handler = MockHandler() + client = Client(handler).client + + class MyModel(BaseModel): + id: str + + plan = Plan(name="my-plan", model=MyModel) + + handler.context.plans = {"my-plan": plan} + response = client.get("/plans") + + assert response.status_code == 200 + assert literal_eval(response.content.decode())["plans"][0] == {"name": "my-plan"} + + +def test_get_plan_by_name() -> None: + handler = MockHandler() + client = Client(handler).client + + class MyModel(BaseModel): + id: str + + plan = Plan(name="my-plan", model=MyModel) + + handler.context.plans = {"my-plan": plan} + response = client.get("/plan/my-plan") + + assert response.status_code == 200 + assert literal_eval(response.content.decode()) == {"name": "my-plan"} + + +def test_get_devices() -> None: + handler = MockHandler() + client = Client(handler).client + + @dataclass + class MyDevice: + name: str + + device = MyDevice("my-device") + + handler.context.devices = {"my-device": device} + response = client.get("/devices") + + assert response.status_code == 200 + assert literal_eval(response.content.decode())["devices"][0] == { + "name": "my-device", + "protocols": ["HasName"], + } + + +def test_get_device_by_name() -> None: + handler = MockHandler() + client = Client(handler).client + + @dataclass + class MyDevice: + name: str + + device = MyDevice("my-device") + + handler.context.devices = {"my-device": device} + response = client.get("/device/my-device") + + assert response.status_code == 200 + assert literal_eval(response.content.decode()) == { + "name": "my-device", + "protocols": ["HasName"], + } + + +def test_put_plan_on_queue() -> None: + handler = MockHandler() + client = Client(handler).client + + client.put("/task/my-task", json={"name": "count", "params": {"detectors": ["x"]}}) + next_task: ActiveTask = handler.worker._task_queue.get(timeout=1.0) + + assert next_task + + +""" +def test_multiple_simultaneous_api_calls_effect_on_get_handler() -> None: + client = TestClient(app) + + # start multiple processes that all do the same thing; get something from the + # client. this tests the lru_cache of get_handler + + # want to check all the processes are successful... + a_thing = multiprocessing.Queue(maxsize=100) + + def test_get_devices(): + print("ah") + response = client.get("/devices") + # a_thing.put(response.status_code) + + process = Process(target=test_get_devices) + process.start() + process.join() + + print("ah") +""" From 2e539f5c1844863dc4b8da6dd115540c379b92b9 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Fri, 5 May 2023 14:16:27 +0000 Subject: [PATCH 03/27] made application compatible with config changes --- pyproject.toml | 1 + src/blueapi/cli/cli.py | 48 +++++++++++++++++++++++----------- src/blueapi/config.py | 7 +++++ src/blueapi/service/handler.py | 37 +++++++++++++------------- tests/test_cli.py | 7 +++++ 5 files changed, 67 insertions(+), 33 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index c90b0e7df..b7e0d750a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,6 +48,7 @@ dev = [ "types-mock", "types-PyYAML", "types-requests", + "types-urllib3", ] [project.scripts] diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index d7997b491..5fd52ea7f 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -1,6 +1,8 @@ import json import logging +from pathlib import Path from pprint import pprint +from typing import Optional import click import requests @@ -8,12 +10,9 @@ from blueapi import __version__ from blueapi.config import ApplicationConfig, ConfigLoader -from blueapi.messaging import StompMessagingTemplate +from blueapi.service.handler import setup_handler from blueapi.service.main import app -from pathlib import Path -from typing import Optional - @click.group(invoke_without_command=True) @click.version_option(version=__version__, prog_name="blueapi") @@ -21,12 +20,13 @@ @click.pass_context def main(ctx, config: Optional[Path]) -> None: # if no command is supplied, run with the options passed + config_loader = ConfigLoader(ApplicationConfig) if config is not None: config_loader.use_values_from_yaml(config) ctx.ensure_object(dict) - ctx.obj["config"] = config_loader.load() + ctx.obj["config_loader"] = config_loader if ctx.invoked_subcommand is None: print("Please invoke subcommand!") @@ -38,8 +38,12 @@ def main(ctx, config: Optional[Path]) -> None: def start_application(obj: dict): import uvicorn - config: ApplicationConfig = obj["config"] - uvicorn.run(app, host=settings.host, port=int(settings.port)) + config_loader: ConfigLoader[ApplicationConfig] = obj["config_loader"] + config = config_loader.load() + + setup_handler(config_loader) + + uvicorn.run(app, host=config.api.host, port=config.api.port) @main.group() @@ -50,7 +54,8 @@ def controller(ctx) -> None: return ctx.ensure_object(dict) - config: ApplicationConfig = ctx.obj["config"] + config_loader: ConfigLoader[ApplicationConfig] = ctx.obj["config_loader"] + config: ApplicationConfig = config_loader.load() logging.basicConfig(level=config.logging.level) @@ -59,23 +64,30 @@ def wrapper(*args, **kwargs): try: func(*args, **kwargs) except ConnectionError: - print("Failed to establish connection.") + print("Failed to establish connection to FastAPI server.") return wrapper @controller.command(name="plans") @check_connection -def get_plans() -> None: - resp = requests.get(f"{settings.url}/plans") +@click.pass_obj +def get_plans(obj: dict) -> None: + config_loader: ConfigLoader[ApplicationConfig] = obj["config_loader"] + config: ApplicationConfig = config_loader.load() + + resp = requests.get(f"http://{config.api.host}:{config.api.port}/plans") print(f"Response returned with {resp.status_code}: ") pprint(resp.json()) @controller.command(name="devices") @check_connection -def get_devices() -> None: - resp = requests.get(f"{settings.url}/devices") +@click.pass_obj +def get_devices(obj: dict) -> None: + config_loader: ConfigLoader[ApplicationConfig] = obj["config_loader"] + config: ApplicationConfig = config_loader.load() + resp = requests.get(f"http://{config.api.host}:{config.api.port}/devices") print(f"Response returned with {resp.status_code}: ") pprint(resp.json()) @@ -84,7 +96,13 @@ def get_devices() -> None: @click.argument("name", type=str) @click.option("-p", "--parameters", type=str, help="Parameters as valid JSON") @check_connection -def run_plan(name: str, parameters: str) -> None: - resp = requests.put(f"{settings.url}/task/{name}", json=json.loads(parameters)) +@click.pass_obj +def run_plan(obj: dict, name: str, parameters: str) -> None: + config_loader: ConfigLoader[ApplicationConfig] = obj["config_loader"] + config: ApplicationConfig = config_loader.load() + resp = requests.put( + f"http://{config.api.host}:{config.api.port}/task/{name}", + json=json.loads(parameters), + ) print(f"Response returned with {resp.status_code}: ") pprint(resp.json()) diff --git a/src/blueapi/config.py b/src/blueapi/config.py index 06446cf3c..4d3af751b 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -35,6 +35,11 @@ class LoggingConfig(BlueapiBaseModel): level: LogLevel = "INFO" +class FastApiConfig(BlueapiBaseModel): + host: str = "localhost" + port: int = 8000 + + class ApplicationConfig(BlueapiBaseModel): """ Config for the worker application as a whole. Root of @@ -44,6 +49,7 @@ class ApplicationConfig(BlueapiBaseModel): stomp: StompConfig = Field(default_factory=StompConfig) env: EnvironmentConfig = Field(default_factory=EnvironmentConfig) logging: LoggingConfig = Field(default_factory=LoggingConfig) + api: FastApiConfig = Field(default_factory=FastApiConfig) def __eq__(self, other: object) -> bool: if isinstance(other, ApplicationConfig): @@ -51,6 +57,7 @@ def __eq__(self, other: object) -> bool: (self.stomp == other.stomp) & (self.env == other.env) & (self.logging == other.logging) + & (self.api == other.api) ) return False diff --git a/src/blueapi/service/handler.py b/src/blueapi/service/handler.py index cd080da83..8b09841d0 100644 --- a/src/blueapi/service/handler.py +++ b/src/blueapi/service/handler.py @@ -1,18 +1,14 @@ import logging from functools import lru_cache -from pathlib import Path from typing import Optional -from blueapi.config import ApplicationConfig, AppSettings +from blueapi.config import ApplicationConfig, ConfigLoader from blueapi.core import BlueskyContext from blueapi.messaging import StompMessagingTemplate from blueapi.messaging.base import MessagingTemplate -from blueapi.utils import ConfigLoader from blueapi.worker.reworker import RunEngineWorker from blueapi.worker.worker import Worker -settings = AppSettings() - class Handler: context: BlueskyContext @@ -20,13 +16,10 @@ class Handler: config: ApplicationConfig message_bus: MessagingTemplate - def __init__(self, config_path: Optional[Path]) -> None: + def __init__(self, config: Optional[ApplicationConfig] = None) -> None: self.context = BlueskyContext() + self.config = config if config is not None else ApplicationConfig() - loader = ConfigLoader(ApplicationConfig) - if config_path is not None: - loader.use_yaml_or_json_file(config_path) - self.config = loader.load() logging.basicConfig(level=self.config.logging.level) self.context.with_startup_script(self.config.env.startup_script) @@ -55,16 +48,24 @@ def stop(self) -> None: self.message_bus.disconnect() -@lru_cache(maxsize=50) -def get_handler() -> Handler: - """Retrieve the handler which wraps the bluesky context, worker and message bus.""" - config_path: Optional[Path] = ( - Path(settings.app_config_path) if settings.app_config_path is not None else None - ) +HANDLER: Optional[Handler] = None - handler = Handler(config_path) + +def setup_handler( + config_loader: Optional[ConfigLoader[ApplicationConfig]] = None, +) -> None: + global HANDLER + handler = Handler(config_loader.load() if config_loader else None) handler.start() - return handler + + HANDLER = handler + + +def get_handler() -> Handler: + """Retrieve the handler which wraps the bluesky context, worker and message bus.""" + if HANDLER is None: + raise ValueError() + return HANDLER def teardown_handler() -> None: diff --git a/tests/test_cli.py b/tests/test_cli.py index bb8a9d22b..5a946a95f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -25,3 +25,10 @@ def test_main_with_nonexistent_config_file(): result.exit_code == 1 type(result.exception) == FileNotFoundError + + +def test_controller_plans(): + runner = CliRunner() + result = runner.invoke(main, ["controller", "plans"]) + + assert result.stdout == "Failed to establish connection to FastAPI server.\n" From db7e3cb974b45a29312365098ea60fdb8e59a5bf Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Fri, 5 May 2023 14:20:12 +0000 Subject: [PATCH 04/27] fixed linting error --- src/blueapi/service/handler.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/blueapi/service/handler.py b/src/blueapi/service/handler.py index 8b09841d0..2903dde31 100644 --- a/src/blueapi/service/handler.py +++ b/src/blueapi/service/handler.py @@ -1,5 +1,4 @@ import logging -from functools import lru_cache from typing import Optional from blueapi.config import ApplicationConfig, ConfigLoader From 16db22af882cd6ec156d8b3de2816b230903fad3 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Fri, 5 May 2023 14:32:35 +0000 Subject: [PATCH 05/27] moved starting the app into blueapi/service/main instead of cli. Added deprecation message for worker. --- src/blueapi/cli/cli.py | 16 +++++++--------- src/blueapi/service/main.py | 12 +++++++++++- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 5fd52ea7f..c30cba5ec 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -10,8 +10,7 @@ from blueapi import __version__ from blueapi.config import ApplicationConfig, ConfigLoader -from blueapi.service.handler import setup_handler -from blueapi.service.main import app +from blueapi.service.main import start @click.group(invoke_without_command=True) @@ -32,18 +31,17 @@ def main(ctx, config: Optional[Path]) -> None: print("Please invoke subcommand!") -@click.version_option(version=__version__) @main.command(name="run") @click.pass_obj def start_application(obj: dict): - import uvicorn + start(obj["config_loader"]) - config_loader: ConfigLoader[ApplicationConfig] = obj["config_loader"] - config = config_loader.load() - - setup_handler(config_loader) - uvicorn.run(app, host=config.api.host, port=config.api.port) +@main.command(name="worker", deprecated=True) +@click.pass_obj +def deprecated_start_application(obj: dict): + print("Please use run command instead.\n") + start(obj["config_loader"]) @main.group() diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 01fd8b9ec..3f6ca613c 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -1,8 +1,9 @@ from fastapi import Body, Depends, FastAPI, HTTPException +from blueapi.config import ApplicationConfig, ConfigLoader from blueapi.worker import RunPlan -from .handler import Handler, get_handler, teardown_handler +from .handler import Handler, get_handler, setup_handler, teardown_handler from .model import DeviceModel, DeviceResponse, PlanModel, PlanResponse app = FastAPI( @@ -54,3 +55,12 @@ async def execute_task( # basically in here, do the same thing the service once did... handler.worker.submit_task(name, task) pass + + +def start(config_loader: ConfigLoader[ApplicationConfig]): + import uvicorn + + config = config_loader.load() + setup_handler(config_loader) + + uvicorn.run(app, host=config.api.host, port=config.api.port) From c2670dd9939e649a24af3c59dc8f1bececcdd422 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Fri, 5 May 2023 14:33:09 +0000 Subject: [PATCH 06/27] moved starting the app into blueapi/service/main instead of cli. Added deprecation message for worker. --- src/blueapi/service/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 3f6ca613c..5a25be9bd 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -1,6 +1,6 @@ from fastapi import Body, Depends, FastAPI, HTTPException -from blueapi.config import ApplicationConfig, ConfigLoader +from blueapi.config import ApplicationConfig, ConfigLoader from blueapi.worker import RunPlan from .handler import Handler, get_handler, setup_handler, teardown_handler From 156d4185ad4f1f0864790d3899857116ab2d2edf Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Fri, 5 May 2023 14:48:25 +0000 Subject: [PATCH 07/27] responded to comments --- .vscode/launch.json | 6 +++--- src/blueapi/cli/cli.py | 20 +++++++++----------- src/blueapi/config.py | 4 ++-- src/blueapi/service/handler.py | 6 +++--- src/blueapi/service/main.py | 11 ++++------- 5 files changed, 21 insertions(+), 26 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 2fb4dc60b..dae07faa7 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -5,7 +5,7 @@ "version": "0.2.0", "configurations": [ { - "name": "Python: FastAPI", + "name": "Debug Rest Service", "type": "python", "request": "launch", "module": "uvicorn", @@ -34,13 +34,13 @@ }, }, { - "name": "Worker Service", + "name": "Run Service", "type": "python", "request": "launch", "justMyCode": false, "module": "blueapi.cli", "args": [ - "worker" + "run" ] }, { diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index c30cba5ec..ce230f75f 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -25,7 +25,7 @@ def main(ctx, config: Optional[Path]) -> None: config_loader.use_values_from_yaml(config) ctx.ensure_object(dict) - ctx.obj["config_loader"] = config_loader + ctx.obj["config"] = config_loader.load() if ctx.invoked_subcommand is None: print("Please invoke subcommand!") @@ -34,14 +34,14 @@ def main(ctx, config: Optional[Path]) -> None: @main.command(name="run") @click.pass_obj def start_application(obj: dict): - start(obj["config_loader"]) + start(obj["config"]) @main.command(name="worker", deprecated=True) @click.pass_obj def deprecated_start_application(obj: dict): print("Please use run command instead.\n") - start(obj["config_loader"]) + start(obj["config"]) @main.group() @@ -52,8 +52,7 @@ def controller(ctx) -> None: return ctx.ensure_object(dict) - config_loader: ConfigLoader[ApplicationConfig] = ctx.obj["config_loader"] - config: ApplicationConfig = config_loader.load() + config: ApplicationConfig = ctx.obj["config"] logging.basicConfig(level=config.logging.level) @@ -71,8 +70,7 @@ def wrapper(*args, **kwargs): @check_connection @click.pass_obj def get_plans(obj: dict) -> None: - config_loader: ConfigLoader[ApplicationConfig] = obj["config_loader"] - config: ApplicationConfig = config_loader.load() + config: ApplicationConfig = obj["config"] resp = requests.get(f"http://{config.api.host}:{config.api.port}/plans") print(f"Response returned with {resp.status_code}: ") @@ -83,8 +81,8 @@ def get_plans(obj: dict) -> None: @check_connection @click.pass_obj def get_devices(obj: dict) -> None: - config_loader: ConfigLoader[ApplicationConfig] = obj["config_loader"] - config: ApplicationConfig = config_loader.load() + config: ApplicationConfig = obj["config"] + resp = requests.get(f"http://{config.api.host}:{config.api.port}/devices") print(f"Response returned with {resp.status_code}: ") pprint(resp.json()) @@ -96,8 +94,8 @@ def get_devices(obj: dict) -> None: @check_connection @click.pass_obj def run_plan(obj: dict, name: str, parameters: str) -> None: - config_loader: ConfigLoader[ApplicationConfig] = obj["config_loader"] - config: ApplicationConfig = config_loader.load() + config: ApplicationConfig = obj["config"] + resp = requests.put( f"http://{config.api.host}:{config.api.port}/task/{name}", json=json.loads(parameters), diff --git a/src/blueapi/config.py b/src/blueapi/config.py index 4d3af751b..3e7dce457 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -35,7 +35,7 @@ class LoggingConfig(BlueapiBaseModel): level: LogLevel = "INFO" -class FastApiConfig(BlueapiBaseModel): +class RestConfig(BlueapiBaseModel): host: str = "localhost" port: int = 8000 @@ -49,7 +49,7 @@ class ApplicationConfig(BlueapiBaseModel): stomp: StompConfig = Field(default_factory=StompConfig) env: EnvironmentConfig = Field(default_factory=EnvironmentConfig) logging: LoggingConfig = Field(default_factory=LoggingConfig) - api: FastApiConfig = Field(default_factory=FastApiConfig) + api: RestConfig = Field(default_factory=RestConfig) def __eq__(self, other: object) -> bool: if isinstance(other, ApplicationConfig): diff --git a/src/blueapi/service/handler.py b/src/blueapi/service/handler.py index 2903dde31..e4910400a 100644 --- a/src/blueapi/service/handler.py +++ b/src/blueapi/service/handler.py @@ -1,7 +1,7 @@ import logging from typing import Optional -from blueapi.config import ApplicationConfig, ConfigLoader +from blueapi.config import ApplicationConfig from blueapi.core import BlueskyContext from blueapi.messaging import StompMessagingTemplate from blueapi.messaging.base import MessagingTemplate @@ -51,10 +51,10 @@ def stop(self) -> None: def setup_handler( - config_loader: Optional[ConfigLoader[ApplicationConfig]] = None, + config: Optional[ApplicationConfig] = None, ) -> None: global HANDLER - handler = Handler(config_loader.load() if config_loader else None) + handler = Handler(config) handler.start() HANDLER = handler diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 5a25be9bd..aa1adadb9 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -1,14 +1,12 @@ from fastapi import Body, Depends, FastAPI, HTTPException -from blueapi.config import ApplicationConfig, ConfigLoader +from blueapi.config import ApplicationConfig from blueapi.worker import RunPlan from .handler import Handler, get_handler, setup_handler, teardown_handler from .model import DeviceModel, DeviceResponse, PlanModel, PlanResponse -app = FastAPI( - docs_url="/docs", on_shutdown=[teardown_handler], on_startup=[get_handler] -) +app = FastAPI(docs_url="/docs", on_shutdown=[teardown_handler]) @app.get("/plans", response_model=PlanResponse) @@ -57,10 +55,9 @@ async def execute_task( pass -def start(config_loader: ConfigLoader[ApplicationConfig]): +def start(config: ApplicationConfig): import uvicorn - config = config_loader.load() - setup_handler(config_loader) + setup_handler(config) uvicorn.run(app, host=config.api.host, port=config.api.port) From c40704312f6053aaa3f196182bc3158fed0ddfb7 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Fri, 5 May 2023 16:03:22 +0000 Subject: [PATCH 08/27] added some cli tests. Intend to add more... --- tests/service/test_rest_api.py | 23 --------- tests/test_cli.py | 92 +++++++++++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 25 deletions(-) diff --git a/tests/service/test_rest_api.py b/tests/service/test_rest_api.py index 08d5cf86a..8607c7c39 100644 --- a/tests/service/test_rest_api.py +++ b/tests/service/test_rest_api.py @@ -114,26 +114,3 @@ def test_put_plan_on_queue() -> None: next_task: ActiveTask = handler.worker._task_queue.get(timeout=1.0) assert next_task - - -""" -def test_multiple_simultaneous_api_calls_effect_on_get_handler() -> None: - client = TestClient(app) - - # start multiple processes that all do the same thing; get something from the - # client. this tests the lru_cache of get_handler - - # want to check all the processes are successful... - a_thing = multiprocessing.Queue(maxsize=100) - - def test_get_devices(): - print("ah") - response = client.get("/devices") - # a_thing.put(response.status_code) - - process = Process(target=test_get_devices) - process.start() - process.join() - - print("ah") -""" diff --git a/tests/test_cli.py b/tests/test_cli.py index 5a946a95f..b66ef4f55 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,11 +1,24 @@ +import pytest from click.testing import CliRunner +from fastapi.testclient import TestClient +from mock import Mock, patch +from pydantic import BaseModel from blueapi import __version__ from blueapi.cli.cli import main +from blueapi.core.bluesky_types import Plan +from blueapi.core.context import BlueskyContext +from blueapi.service.handler import get_handler +from blueapi.service.main import app +from blueapi.worker.reworker import RunEngineWorker -def test_cli_version(): - runner = CliRunner() +@pytest.fixture +def runner(): + return CliRunner() + + +def test_cli_version(runner: CliRunner): result = runner.invoke(main, ["--version"]) assert result.stdout == f"blueapi, version {__version__}\n" @@ -32,3 +45,78 @@ def test_controller_plans(): result = runner.invoke(main, ["controller", "plans"]) assert result.stdout == "Failed to establish connection to FastAPI server.\n" + + +# Some CLI commands require the rest api to be running... + + +class MockHandler: + context: BlueskyContext + worker: RunEngineWorker + + def __init__(self) -> None: + self.context = BlueskyContext() + self.worker = RunEngineWorker(self.context) + + def start(self): + return None + + +class Client: + def __init__(self, handler: MockHandler) -> None: + """Create tester object""" + self.handler = handler + + @property + def client(self) -> TestClient: + app.dependency_overrides[get_handler] = lambda: self.handler + return TestClient(app) + + +@patch("blueapi.service.handler.Handler", autospec=True) +@patch("uvicorn.run", side_effect=[None]) +def test_deprecated_worker_command(mock_handler, mock_uvicorn, runner: CliRunner): + dummy = Mock() + dummy.return_value = MockHandler() + mock_handler.side_effect = [dummy] + + result = runner.invoke(main, ["worker"]) + + assert result.output == ( + "DeprecationWarning: The command 'worker' is deprecated.\n" + + "Please use run command instead.\n\n" + ) + + +@patch("blueapi.service.handler.Handler") +@patch("requests.get") +def test_get_plans(mock_requests, mock_handler, runner: CliRunner): + # 1. "start" the server as above... (not really started as uvicorn not run, + # but handler set.) + # 2. add something manually onto the handler and check if you can get it + # via the cli. Mock out requests.get with TestClient calls. + + handler = MockHandler() + + dummy = Mock() + dummy.return_value = handler + mock_handler.side_effect = [dummy] + + with patch("uvicorn.run", side_effect=[None]): + result = runner.invoke(main, ["run"]) + + assert result.exit_code == 0 + + class MyModel(BaseModel): + id: str + + plan = Plan(name="my-plan", model=MyModel) + + handler.context.plans = {"my-plan": plan} + + mock_requests.return_value = Client(handler).client.get("/plans") + + plans = runner.invoke(main, ["controller", "plans"]) + assert plans.output == ( + "Response returned with 200: " + "\n{'plans': [{'name': 'my-plan'}]}\n" + ) From f65425e084f8fd5c644e59be0b6ba909df06f66a Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Fri, 5 May 2023 16:05:54 +0000 Subject: [PATCH 09/27] added mock as dependency to pass tests --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index b7e0d750a..877760438 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -49,6 +49,7 @@ dev = [ "types-PyYAML", "types-requests", "types-urllib3", + "mock", ] [project.scripts] From 236a08f3510fb1afe592f6beff6e221e69eb59d3 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Tue, 9 May 2023 09:36:22 +0000 Subject: [PATCH 10/27] added more tests for cli and modified existing rest tests --- tests/service/test_rest_api.py | 2 - tests/test_cli.py | 91 ++++++++++++++++++++++++++++++---- 2 files changed, 80 insertions(+), 13 deletions(-) diff --git a/tests/service/test_rest_api.py b/tests/service/test_rest_api.py index 8607c7c39..42583fff2 100644 --- a/tests/service/test_rest_api.py +++ b/tests/service/test_rest_api.py @@ -11,8 +11,6 @@ from blueapi.worker import RunEngineWorker from blueapi.worker.task import ActiveTask -# client = TestClient(app) - class MockHandler: context: BlueskyContext diff --git a/tests/test_cli.py b/tests/test_cli.py index b66ef4f55..1c0122191 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,3 +1,5 @@ +from dataclasses import dataclass + import pytest from click.testing import CliRunner from fastapi.testclient import TestClient @@ -11,6 +13,7 @@ from blueapi.service.handler import get_handler from blueapi.service.main import app from blueapi.worker.reworker import RunEngineWorker +from blueapi.worker.task import ActiveTask @pytest.fixture @@ -73,6 +76,15 @@ def client(self) -> TestClient: return TestClient(app) +class MyModel(BaseModel): + id: str + + +@dataclass +class MyDevice: + name: str + + @patch("blueapi.service.handler.Handler", autospec=True) @patch("uvicorn.run", side_effect=[None]) def test_deprecated_worker_command(mock_handler, mock_uvicorn, runner: CliRunner): @@ -90,11 +102,22 @@ def test_deprecated_worker_command(mock_handler, mock_uvicorn, runner: CliRunner @patch("blueapi.service.handler.Handler") @patch("requests.get") -def test_get_plans(mock_requests, mock_handler, runner: CliRunner): - # 1. "start" the server as above... (not really started as uvicorn not run, - # but handler set.) - # 2. add something manually onto the handler and check if you can get it - # via the cli. Mock out requests.get with TestClient calls. +def test_get_plans_and_devices(mock_requests, mock_handler, runner: CliRunner): + """Integration test which attempts to test a couple of CLI commands. + + This test mocks out the handler so that setup_handler (which gets called at the + start of the application when the CLI command `blueapi run` is executed) actually + sets up a handler I can directly add things to, e.g. plans and devices. + In reality, at this stage the bluesky worker would be started and a connection + to activemq setup. However, the mocked handler does not do this for simplicity's + sake. + + This test also mocks out the calls to rest API endpoints with calls to a + TestClient instance for FastAPI. + + The CliRunner fixture passed to this test simply runs the CLI commands passed to + it. + """ handler = MockHandler() @@ -107,16 +130,62 @@ def test_get_plans(mock_requests, mock_handler, runner: CliRunner): assert result.exit_code == 0 - class MyModel(BaseModel): - id: str - plan = Plan(name="my-plan", model=MyModel) - handler.context.plans = {"my-plan": plan} - mock_requests.return_value = Client(handler).client.get("/plans") - plans = runner.invoke(main, ["controller", "plans"]) + + mock_requests.return_value = Client(handler).client.get("/devices") + unset_devices = runner.invoke(main, ["controller", "devices"]) + assert unset_devices.output == "Response returned with 200: \n{'devices': []}\n" + + device = MyDevice("my-device") + handler.context.devices = {"my-device": device} + mock_requests.return_value = Client(handler).client.get("/devices") + devices = runner.invoke(main, ["controller", "devices"]) + assert plans.output == ( "Response returned with 200: " + "\n{'plans': [{'name': 'my-plan'}]}\n" ) + assert devices.output == ( + "Response returned with 200: " + + "\n{'devices': [{'name': 'my-device', 'protocols': ['HasName']}]}\n" + ) + + +@patch("blueapi.service.handler.Handler") +@patch("requests.get") +def test_run_plan_through_cli(mock_requests, mock_handler, runner: CliRunner): + """Integration test which attempts to put a plan on the worker queue. + + This test mocks out the handler so that setup_handler (which gets called at the + start of the application when the CLI command `blueapi run` is executed) actually + sets up a handler I can directly add things to, e.g. plans and devices. + In reality, at this stage the bluesky worker would be started and a connection + to activemq setup. However, the mocked handler does not do this for simplicity's + sake. + + This test also mocks out the calls to rest API endpoints with calls to a + TestClient instance for FastAPI. + + The CliRunner fixture passed to this test simply runs the CLI commands passed to + it. + """ + + handler = MockHandler() + + dummy = Mock() + dummy.return_value = handler + mock_handler.side_effect = [dummy] + + with patch("uvicorn.run", side_effect=[None]): + result = runner.invoke(main, ["run"]) + + assert result.exit_code == 0 + + mock_requests.return_value = Client(handler).client.put( + "/task/my-task", json={"name": "count", "params": {"detectors": ["x"]}} + ) + next_task: ActiveTask = handler.worker._task_queue.get(timeout=1.0) + + assert next_task From a390a09f8de16aa5999353fdca82c04d86ab9b38 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Tue, 9 May 2023 10:40:25 +0000 Subject: [PATCH 11/27] changed run to serve instead --- .devcontainer/Dockerfile | 2 +- .vscode/launch.json | 2 +- src/blueapi/cli/cli.py | 4 ++-- tests/test_cli.py | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index b669993ac..50a7dfccb 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -34,4 +34,4 @@ ENV PATH=/venv/bin:$PATH # change this entrypoint if it is not the same as the repo ENTRYPOINT ["blueapi"] -CMD ["run"] +CMD ["serve"] diff --git a/.vscode/launch.json b/.vscode/launch.json index dae07faa7..9d2529db5 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -40,7 +40,7 @@ "justMyCode": false, "module": "blueapi.cli", "args": [ - "run" + "serve" ] }, { diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index ce230f75f..a5006b207 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -31,7 +31,7 @@ def main(ctx, config: Optional[Path]) -> None: print("Please invoke subcommand!") -@main.command(name="run") +@main.command(name="serve") @click.pass_obj def start_application(obj: dict): start(obj["config"]) @@ -88,7 +88,7 @@ def get_devices(obj: dict) -> None: pprint(resp.json()) -@controller.command(name="run") +@controller.command(name="serve") @click.argument("name", type=str) @click.option("-p", "--parameters", type=str, help="Parameters as valid JSON") @check_connection diff --git a/tests/test_cli.py b/tests/test_cli.py index 1c0122191..0667a83ff 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -126,7 +126,7 @@ def test_get_plans_and_devices(mock_requests, mock_handler, runner: CliRunner): mock_handler.side_effect = [dummy] with patch("uvicorn.run", side_effect=[None]): - result = runner.invoke(main, ["run"]) + result = runner.invoke(main, ["serve"]) assert result.exit_code == 0 @@ -179,7 +179,7 @@ def test_run_plan_through_cli(mock_requests, mock_handler, runner: CliRunner): mock_handler.side_effect = [dummy] with patch("uvicorn.run", side_effect=[None]): - result = runner.invoke(main, ["run"]) + result = runner.invoke(main, ["serve"]) assert result.exit_code == 0 From 0f8f45a77a420111953e24063ce48ce9297738be Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Tue, 9 May 2023 10:48:56 +0000 Subject: [PATCH 12/27] made teardown handler do nothing if handler not set --- src/blueapi/service/handler.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/blueapi/service/handler.py b/src/blueapi/service/handler.py index e4910400a..438d93aa6 100644 --- a/src/blueapi/service/handler.py +++ b/src/blueapi/service/handler.py @@ -69,5 +69,7 @@ def get_handler() -> Handler: def teardown_handler() -> None: """Stop all handler tasks. Does nothing if setup_handler has not been called.""" + if HANDLER is None: + return handler = get_handler() handler.stop() From 68b733040973c4901df4bad84be282fc86c439a8 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Tue, 9 May 2023 10:51:02 +0000 Subject: [PATCH 13/27] removed redundant comment --- src/blueapi/service/main.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index aa1adadb9..d3b46a309 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -50,7 +50,6 @@ async def execute_task( ), handler: Handler = Depends(get_handler), ): - # basically in here, do the same thing the service once did... handler.worker.submit_task(name, task) pass From 1a2621365aedbd1afdf09291ef52be479bc8500b Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Tue, 9 May 2023 11:16:26 +0000 Subject: [PATCH 14/27] removed redundant string concatenation in test --- tests/test_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 0667a83ff..22098df6d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -145,7 +145,7 @@ def test_get_plans_and_devices(mock_requests, mock_handler, runner: CliRunner): devices = runner.invoke(main, ["controller", "devices"]) assert plans.output == ( - "Response returned with 200: " + "\n{'plans': [{'name': 'my-plan'}]}\n" + "Response returned with 200: \n{'plans': [{'name': 'my-plan'}]}\n" ) assert devices.output == ( "Response returned with 200: " From e9cccbe8b9d131a303540d62047bc1ca9e1609a4 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Tue, 9 May 2023 11:18:14 +0000 Subject: [PATCH 15/27] moved assert statement --- tests/test_cli.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 22098df6d..7bdae1359 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -135,6 +135,10 @@ def test_get_plans_and_devices(mock_requests, mock_handler, runner: CliRunner): mock_requests.return_value = Client(handler).client.get("/plans") plans = runner.invoke(main, ["controller", "plans"]) + assert plans.output == ( + "Response returned with 200: \n{'plans': [{'name': 'my-plan'}]}\n" + ) + mock_requests.return_value = Client(handler).client.get("/devices") unset_devices = runner.invoke(main, ["controller", "devices"]) assert unset_devices.output == "Response returned with 200: \n{'devices': []}\n" @@ -144,9 +148,6 @@ def test_get_plans_and_devices(mock_requests, mock_handler, runner: CliRunner): mock_requests.return_value = Client(handler).client.get("/devices") devices = runner.invoke(main, ["controller", "devices"]) - assert plans.output == ( - "Response returned with 200: \n{'plans': [{'name': 'my-plan'}]}\n" - ) assert devices.output == ( "Response returned with 200: " + "\n{'devices': [{'name': 'my-device', 'protocols': ['HasName']}]}\n" From a702f5fe2e7538db7ae2b3f489f64a2d9abdf35a Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Tue, 9 May 2023 12:17:32 +0000 Subject: [PATCH 16/27] made minor changes in response to comments; using functools wraps and .json() instead of literal_eval in tests --- src/blueapi/cli/cli.py | 2 ++ tests/service/test_rest_api.py | 17 ++++++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index a5006b207..bfdeb1e31 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -1,5 +1,6 @@ import json import logging +from functools import wraps from pathlib import Path from pprint import pprint from typing import Optional @@ -57,6 +58,7 @@ def controller(ctx) -> None: def check_connection(func): + @wraps(func) def wrapper(*args, **kwargs): try: func(*args, **kwargs) diff --git a/tests/service/test_rest_api.py b/tests/service/test_rest_api.py index 42583fff2..d7d7af83a 100644 --- a/tests/service/test_rest_api.py +++ b/tests/service/test_rest_api.py @@ -1,4 +1,3 @@ -from ast import literal_eval from dataclasses import dataclass from fastapi.testclient import TestClient @@ -45,7 +44,7 @@ class MyModel(BaseModel): response = client.get("/plans") assert response.status_code == 200 - assert literal_eval(response.content.decode())["plans"][0] == {"name": "my-plan"} + assert response.json() == {"plans": [{"name": "my-plan"}]} def test_get_plan_by_name() -> None: @@ -61,7 +60,7 @@ class MyModel(BaseModel): response = client.get("/plan/my-plan") assert response.status_code == 200 - assert literal_eval(response.content.decode()) == {"name": "my-plan"} + assert response.json() == {"name": "my-plan"} def test_get_devices() -> None: @@ -78,9 +77,13 @@ class MyDevice: response = client.get("/devices") assert response.status_code == 200 - assert literal_eval(response.content.decode())["devices"][0] == { - "name": "my-device", - "protocols": ["HasName"], + assert response.json() == { + "devices": [ + { + "name": "my-device", + "protocols": ["HasName"], + } + ] } @@ -98,7 +101,7 @@ class MyDevice: response = client.get("/device/my-device") assert response.status_code == 200 - assert literal_eval(response.content.decode()) == { + assert response.json() == { "name": "my-device", "protocols": ["HasName"], } From 5450bd1bdf117e247a9cf553d0742f801c88c1f5 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Tue, 9 May 2023 15:45:34 +0000 Subject: [PATCH 17/27] fixed minor issues; param ingestion for cli.py::run_plan and made fixtures for tests --- src/blueapi/cli/cli.py | 13 +-- src/blueapi/cli/fake_cli.py | 38 ++++++++ src/blueapi/service/main.py | 9 +- tests/example_yaml/rest_config.yaml | 3 + tests/service/test_rest_api.py | 54 +++++++----- tests/test_cli.py | 131 +++++++++++++++------------- 6 files changed, 151 insertions(+), 97 deletions(-) create mode 100644 src/blueapi/cli/fake_cli.py create mode 100644 tests/example_yaml/rest_config.yaml diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index bfdeb1e31..c8c83fa44 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -18,12 +18,15 @@ @click.version_option(version=__version__, prog_name="blueapi") @click.option("-c", "--config", type=Path, help="Path to configuration YAML file") @click.pass_context -def main(ctx, config: Optional[Path]) -> None: +def main(ctx: click.Context, config: Optional[Path]) -> None: # if no command is supplied, run with the options passed config_loader = ConfigLoader(ApplicationConfig) if config is not None: - config_loader.use_values_from_yaml(config) + if config.exists(): + config_loader.use_values_from_yaml(config) + else: + raise FileNotFoundError(f"Cannot find file: {config}") ctx.ensure_object(dict) ctx.obj["config"] = config_loader.load() @@ -47,7 +50,7 @@ def deprecated_start_application(obj: dict): @main.group() @click.pass_context -def controller(ctx) -> None: +def controller(ctx: click.Context) -> None: if ctx.invoked_subcommand is None: print("Please invoke subcommand!") return @@ -90,7 +93,7 @@ def get_devices(obj: dict) -> None: pprint(resp.json()) -@controller.command(name="serve") +@controller.command(name="run") @click.argument("name", type=str) @click.option("-p", "--parameters", type=str, help="Parameters as valid JSON") @check_connection @@ -100,7 +103,7 @@ def run_plan(obj: dict, name: str, parameters: str) -> None: resp = requests.put( f"http://{config.api.host}:{config.api.port}/task/{name}", - json=json.loads(parameters), + json={"name": name, "params": json.loads(parameters)}, ) print(f"Response returned with {resp.status_code}: ") pprint(resp.json()) diff --git a/src/blueapi/cli/fake_cli.py b/src/blueapi/cli/fake_cli.py new file mode 100644 index 000000000..129fe547b --- /dev/null +++ b/src/blueapi/cli/fake_cli.py @@ -0,0 +1,38 @@ +import os + +import click + + +class Repo(object): + def __init__(self, home=None, debug=False): + self.home = os.path.abspath(home or ".") + self.debug = debug + + +@click.group() +@click.option("--repo-home", envvar="REPO_HOME", default=".repo") +@click.option("--debug/--no-debug", default=False, envvar="REPO_DEBUG") +@click.pass_context +def cli(ctx, repo_home, debug): + ctx.obj = Repo(repo_home, debug) + + +pass_repo = click.make_pass_decorator(Repo) + + +@cli.command() +@click.argument("src") +@click.argument("dest", required=False) +@pass_repo +def clone(repo, src, dest): + pass + + +@cli.command() +@pass_repo +def cp(repo): + click.echo(isinstance(repo, Repo)) + + +if __name__ == "__main__": + cli() diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index d3b46a309..db38af02b 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -1,3 +1,5 @@ +from typing import Any, Mapping + from fastapi import Body, Depends, FastAPI, HTTPException from blueapi.config import ApplicationConfig @@ -45,13 +47,10 @@ async def get_device_by_name(name: str, handler: Handler = Depends(get_handler)) @app.put("/task/{name}") async def execute_task( name: str, - task: RunPlan = Body( - ..., example={"name": "count", "params": {"detectors": ["x"]}} - ), + task: Mapping[str, Any] = Body(..., example={"detectors": ["x"]}), handler: Handler = Depends(get_handler), ): - handler.worker.submit_task(name, task) - pass + handler.worker.submit_task(name, RunPlan(name=name, params=task)) def start(config: ApplicationConfig): diff --git a/tests/example_yaml/rest_config.yaml b/tests/example_yaml/rest_config.yaml new file mode 100644 index 000000000..51a4714b1 --- /dev/null +++ b/tests/example_yaml/rest_config.yaml @@ -0,0 +1,3 @@ +api: + host: a.fake.host + port: 12345 diff --git a/tests/service/test_rest_api.py b/tests/service/test_rest_api.py index d7d7af83a..5f8d73f20 100644 --- a/tests/service/test_rest_api.py +++ b/tests/service/test_rest_api.py @@ -1,6 +1,8 @@ from dataclasses import dataclass +import pytest from fastapi.testclient import TestClient +from mock import Mock from pydantic import BaseModel from blueapi.core.bluesky_types import Plan @@ -8,7 +10,7 @@ from blueapi.service.handler import get_handler from blueapi.service.main import app from blueapi.worker import RunEngineWorker -from blueapi.worker.task import ActiveTask +from blueapi.worker.task import RunPlan, Task class MockHandler: @@ -16,11 +18,13 @@ class MockHandler: worker: RunEngineWorker def __init__(self) -> None: - self.context = BlueskyContext() - self.worker = RunEngineWorker(self.context) + self.context = Mock() + self.worker = Mock() class Client: + handler = None + def __init__(self, handler: MockHandler) -> None: """Create tester object""" self.handler = handler @@ -31,10 +35,17 @@ def client(self) -> TestClient: return TestClient(app) -def test_get_plans() -> None: - handler = MockHandler() - client = Client(handler).client +@pytest.fixture +def handler() -> MockHandler: + return MockHandler() + + +@pytest.fixture +def client(handler: MockHandler) -> TestClient: + return Client(handler).client + +def test_get_plans(handler: MockHandler, client: TestClient) -> None: class MyModel(BaseModel): id: str @@ -47,10 +58,7 @@ class MyModel(BaseModel): assert response.json() == {"plans": [{"name": "my-plan"}]} -def test_get_plan_by_name() -> None: - handler = MockHandler() - client = Client(handler).client - +def test_get_plan_by_name(handler: MockHandler, client: TestClient) -> None: class MyModel(BaseModel): id: str @@ -63,10 +71,7 @@ class MyModel(BaseModel): assert response.json() == {"name": "my-plan"} -def test_get_devices() -> None: - handler = MockHandler() - client = Client(handler).client - +def test_get_devices(handler: MockHandler, client: TestClient) -> None: @dataclass class MyDevice: name: str @@ -87,10 +92,7 @@ class MyDevice: } -def test_get_device_by_name() -> None: - handler = MockHandler() - client = Client(handler).client - +def test_get_device_by_name(handler: MockHandler, client: TestClient) -> None: @dataclass class MyDevice: name: str @@ -107,11 +109,15 @@ class MyDevice: } -def test_put_plan_on_queue() -> None: - handler = MockHandler() - client = Client(handler).client +def test_put_plan_submits_task(handler: MockHandler, client: TestClient) -> None: + task_json = {"detectors": ["x"]} + task_name = "count" + submitted_tasks = {} + + def on_submit(name: str, task: Task): + submitted_tasks[name] = task - client.put("/task/my-task", json={"name": "count", "params": {"detectors": ["x"]}}) - next_task: ActiveTask = handler.worker._task_queue.get(timeout=1.0) + handler.worker.submit_task.side_effect = on_submit - assert next_task + client.put(f"/task/{task_name}", json=task_json) + assert submitted_tasks == {task_name: RunPlan(name=task_name, params=task_json)} diff --git a/tests/test_cli.py b/tests/test_cli.py index 7bdae1359..b57a795b7 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -13,7 +13,6 @@ from blueapi.service.handler import get_handler from blueapi.service.main import app from blueapi.worker.reworker import RunEngineWorker -from blueapi.worker.task import ActiveTask @pytest.fixture @@ -76,6 +75,16 @@ def client(self) -> TestClient: return TestClient(app) +@pytest.fixture +def handler() -> MockHandler: + return MockHandler() + + +@pytest.fixture +def client(handler: MockHandler) -> TestClient: + return Client(handler).client + + class MyModel(BaseModel): id: str @@ -85,14 +94,14 @@ class MyDevice: name: str -@patch("blueapi.service.handler.Handler", autospec=True) -@patch("uvicorn.run", side_effect=[None]) -def test_deprecated_worker_command(mock_handler, mock_uvicorn, runner: CliRunner): - dummy = Mock() - dummy.return_value = MockHandler() - mock_handler.side_effect = [dummy] +@patch("blueapi.service.handler.Handler") +def test_deprecated_worker_command( + mock_handler: Mock, handler: MockHandler, runner: CliRunner +): + mock_handler.side_effect = Mock(return_value=handler) - result = runner.invoke(main, ["worker"]) + with patch("uvicorn.run", side_effect=None): + result = runner.invoke(main, ["worker"]) assert result.output == ( "DeprecationWarning: The command 'worker' is deprecated.\n" @@ -102,49 +111,49 @@ def test_deprecated_worker_command(mock_handler, mock_uvicorn, runner: CliRunner @patch("blueapi.service.handler.Handler") @patch("requests.get") -def test_get_plans_and_devices(mock_requests, mock_handler, runner: CliRunner): - """Integration test which attempts to test a couple of CLI commands. - - This test mocks out the handler so that setup_handler (which gets called at the - start of the application when the CLI command `blueapi run` is executed) actually - sets up a handler I can directly add things to, e.g. plans and devices. - In reality, at this stage the bluesky worker would be started and a connection - to activemq setup. However, the mocked handler does not do this for simplicity's - sake. - - This test also mocks out the calls to rest API endpoints with calls to a - TestClient instance for FastAPI. - - The CliRunner fixture passed to this test simply runs the CLI commands passed to - it. - """ - - handler = MockHandler() - - dummy = Mock() - dummy.return_value = handler - mock_handler.side_effect = [dummy] - - with patch("uvicorn.run", side_effect=[None]): +def test_get_plans_and_devices( + mock_requests: Mock, + mock_handler: Mock, + handler: MockHandler, + client: TestClient, + runner: CliRunner, +): + """Integration test to test get_plans and get_devices.""" + + # needed so that the handler is instantiated as MockHandler() instead of Handler(). + mock_handler.side_effect = Mock(return_value=handler) + + # Setup the (Mock)Handler. + with patch("uvicorn.run", side_effect=None): result = runner.invoke(main, ["serve"]) assert result.exit_code == 0 + # Put a plan in handler.context manually. plan = Plan(name="my-plan", model=MyModel) handler.context.plans = {"my-plan": plan} - mock_requests.return_value = Client(handler).client.get("/plans") + + # Setup requests.get call to return the output of the FastAPI call for plans. + # Call the CLI function and check the output. + mock_requests.return_value = client.get("/plans") plans = runner.invoke(main, ["controller", "plans"]) assert plans.output == ( "Response returned with 200: \n{'plans': [{'name': 'my-plan'}]}\n" ) - mock_requests.return_value = Client(handler).client.get("/devices") + # Setup requests.get call to return the output of the FastAPI call for devices. + # Call the CLI function and check the output - expect nothing as no devices set. + mock_requests.return_value = client.get("/devices") unset_devices = runner.invoke(main, ["controller", "devices"]) assert unset_devices.output == "Response returned with 200: \n{'devices': []}\n" + # Put a device in handler.context manually. device = MyDevice("my-device") handler.context.devices = {"my-device": device} + + # Setup requests.get call to return the output of the FastAPI call for devices. + # Call the CLI function and check the output. mock_requests.return_value = Client(handler).client.get("/devices") devices = runner.invoke(main, ["controller", "devices"]) @@ -154,39 +163,35 @@ def test_get_plans_and_devices(mock_requests, mock_handler, runner: CliRunner): ) -@patch("blueapi.service.handler.Handler") -@patch("requests.get") -def test_run_plan_through_cli(mock_requests, mock_handler, runner: CliRunner): - """Integration test which attempts to put a plan on the worker queue. - - This test mocks out the handler so that setup_handler (which gets called at the - start of the application when the CLI command `blueapi run` is executed) actually - sets up a handler I can directly add things to, e.g. plans and devices. - In reality, at this stage the bluesky worker would be started and a connection - to activemq setup. However, the mocked handler does not do this for simplicity's - sake. - - This test also mocks out the calls to rest API endpoints with calls to a - TestClient instance for FastAPI. - - The CliRunner fixture passed to this test simply runs the CLI commands passed to - it. - """ - - handler = MockHandler() +def test_invalid_config_path_handling(runner: CliRunner): + # test what happens if you pass an invalid config file... + result = runner.invoke(main, ["-c", "non_existent.yaml"]) + assert result.exit_code == 1 - dummy = Mock() - dummy.return_value = handler - mock_handler.side_effect = [dummy] - with patch("uvicorn.run", side_effect=[None]): - result = runner.invoke(main, ["serve"]) +@patch("blueapi.service.handler.Handler") +@patch("requests.put") +def test_config_passed_down_to_command_children( + mock_requests: Mock, + mock_handler: Mock, + handler: MockHandler, + runner: CliRunner, +): + mock_handler.side_effect = Mock(return_value=handler) + config_path = "tests/example_yaml/rest_config.yaml" + + with patch("uvicorn.run", side_effect=None): + result = runner.invoke(main, ["-c", config_path, "serve"]) assert result.exit_code == 0 - mock_requests.return_value = Client(handler).client.put( - "/task/my-task", json={"name": "count", "params": {"detectors": ["x"]}} + mock_requests.return_value = Mock() + + runner.invoke( + main, ["-c", config_path, "controller", "run", "sleep", "-p", '{"time": 5}'] ) - next_task: ActiveTask = handler.worker._task_queue.get(timeout=1.0) - assert next_task + assert mock_requests.call_args[0][0] == "http://a.fake.host:12345/task/sleep" + assert mock_requests.call_args[1] == { + "json": {"name": "sleep", "params": {"time": 5}} + } From c060e93ef009fd772a961cbbcfffafa7f2bc3b3f Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Tue, 9 May 2023 15:57:16 +0000 Subject: [PATCH 18/27] moved common parts of tests to conftest.py, made session scope fixtures --- tests/conftest.py | 40 +++++++++++++++++++++++++++ tests/service/test_rest_api.py | 49 +++++----------------------------- tests/test_cli.py | 47 +++++--------------------------- 3 files changed, 52 insertions(+), 84 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 0c476303c..8a14ac359 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,14 @@ # Based on https://docs.pytest.org/en/latest/example/simple.html#control-skipping-of-tests-according-to-command-line-option # noqa: E501 +from mock import Mock import pytest +from blueapi.core.context import BlueskyContext +from blueapi.worker.reworker import RunEngineWorker +from blueapi.service.handler import Handler, get_handler +from blueapi.service.main import app +from fastapi.testclient import TestClient + def pytest_addoption(parser): parser.addoption( @@ -22,3 +29,36 @@ def pytest_collection_modifyitems(config, items): for item in items: if "stomp" in item.keywords: item.add_marker(skip_stomp) + + +class MockHandler(Handler): + context: BlueskyContext + worker: RunEngineWorker + + def __init__(self) -> None: + self.context = Mock() + self.worker = Mock() + + def start(self): + return None + + +class Client: + def __init__(self, handler: MockHandler) -> None: + """Create tester object""" + self.handler = handler + + @property + def client(self) -> TestClient: + app.dependency_overrides[get_handler] = lambda: self.handler + return TestClient(app) + + +@pytest.fixture(scope="session") +def handler() -> MockHandler: + return MockHandler() + + +@pytest.fixture(scope="session") +def client(handler: MockHandler) -> TestClient: + return Client(handler).client diff --git a/tests/service/test_rest_api.py b/tests/service/test_rest_api.py index 5f8d73f20..a488198e8 100644 --- a/tests/service/test_rest_api.py +++ b/tests/service/test_rest_api.py @@ -1,51 +1,14 @@ from dataclasses import dataclass -import pytest from fastapi.testclient import TestClient -from mock import Mock from pydantic import BaseModel from blueapi.core.bluesky_types import Plan -from blueapi.core.context import BlueskyContext -from blueapi.service.handler import get_handler -from blueapi.service.main import app -from blueapi.worker import RunEngineWorker +from blueapi.service.handler import Handler from blueapi.worker.task import RunPlan, Task -class MockHandler: - context: BlueskyContext - worker: RunEngineWorker - - def __init__(self) -> None: - self.context = Mock() - self.worker = Mock() - - -class Client: - handler = None - - def __init__(self, handler: MockHandler) -> None: - """Create tester object""" - self.handler = handler - - @property - def client(self) -> TestClient: - app.dependency_overrides[get_handler] = lambda: self.handler - return TestClient(app) - - -@pytest.fixture -def handler() -> MockHandler: - return MockHandler() - - -@pytest.fixture -def client(handler: MockHandler) -> TestClient: - return Client(handler).client - - -def test_get_plans(handler: MockHandler, client: TestClient) -> None: +def test_get_plans(handler: Handler, client: TestClient) -> None: class MyModel(BaseModel): id: str @@ -58,7 +21,7 @@ class MyModel(BaseModel): assert response.json() == {"plans": [{"name": "my-plan"}]} -def test_get_plan_by_name(handler: MockHandler, client: TestClient) -> None: +def test_get_plan_by_name(handler: Handler, client: TestClient) -> None: class MyModel(BaseModel): id: str @@ -71,7 +34,7 @@ class MyModel(BaseModel): assert response.json() == {"name": "my-plan"} -def test_get_devices(handler: MockHandler, client: TestClient) -> None: +def test_get_devices(handler: Handler, client: TestClient) -> None: @dataclass class MyDevice: name: str @@ -92,7 +55,7 @@ class MyDevice: } -def test_get_device_by_name(handler: MockHandler, client: TestClient) -> None: +def test_get_device_by_name(handler: Handler, client: TestClient) -> None: @dataclass class MyDevice: name: str @@ -109,7 +72,7 @@ class MyDevice: } -def test_put_plan_submits_task(handler: MockHandler, client: TestClient) -> None: +def test_put_plan_submits_task(handler: Handler, client: TestClient) -> None: task_json = {"detectors": ["x"]} task_name = "count" submitted_tasks = {} diff --git a/tests/test_cli.py b/tests/test_cli.py index b57a795b7..d6b0a8f0c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -9,10 +9,7 @@ from blueapi import __version__ from blueapi.cli.cli import main from blueapi.core.bluesky_types import Plan -from blueapi.core.context import BlueskyContext -from blueapi.service.handler import get_handler -from blueapi.service.main import app -from blueapi.worker.reworker import RunEngineWorker +from blueapi.service.handler import Handler @pytest.fixture @@ -52,39 +49,6 @@ def test_controller_plans(): # Some CLI commands require the rest api to be running... -class MockHandler: - context: BlueskyContext - worker: RunEngineWorker - - def __init__(self) -> None: - self.context = BlueskyContext() - self.worker = RunEngineWorker(self.context) - - def start(self): - return None - - -class Client: - def __init__(self, handler: MockHandler) -> None: - """Create tester object""" - self.handler = handler - - @property - def client(self) -> TestClient: - app.dependency_overrides[get_handler] = lambda: self.handler - return TestClient(app) - - -@pytest.fixture -def handler() -> MockHandler: - return MockHandler() - - -@pytest.fixture -def client(handler: MockHandler) -> TestClient: - return Client(handler).client - - class MyModel(BaseModel): id: str @@ -96,7 +60,7 @@ class MyDevice: @patch("blueapi.service.handler.Handler") def test_deprecated_worker_command( - mock_handler: Mock, handler: MockHandler, runner: CliRunner + mock_handler: Mock, handler: Handler, runner: CliRunner ): mock_handler.side_effect = Mock(return_value=handler) @@ -114,7 +78,7 @@ def test_deprecated_worker_command( def test_get_plans_and_devices( mock_requests: Mock, mock_handler: Mock, - handler: MockHandler, + handler: Handler, client: TestClient, runner: CliRunner, ): @@ -144,6 +108,7 @@ def test_get_plans_and_devices( # Setup requests.get call to return the output of the FastAPI call for devices. # Call the CLI function and check the output - expect nothing as no devices set. + handler.context.devices = {} mock_requests.return_value = client.get("/devices") unset_devices = runner.invoke(main, ["controller", "devices"]) assert unset_devices.output == "Response returned with 200: \n{'devices': []}\n" @@ -154,7 +119,7 @@ def test_get_plans_and_devices( # Setup requests.get call to return the output of the FastAPI call for devices. # Call the CLI function and check the output. - mock_requests.return_value = Client(handler).client.get("/devices") + mock_requests.return_value = client.get("/devices") devices = runner.invoke(main, ["controller", "devices"]) assert devices.output == ( @@ -174,7 +139,7 @@ def test_invalid_config_path_handling(runner: CliRunner): def test_config_passed_down_to_command_children( mock_requests: Mock, mock_handler: Mock, - handler: MockHandler, + handler: Handler, runner: CliRunner, ): mock_handler.side_effect = Mock(return_value=handler) From 6d1119ad80319ccb86d47d88104c19486d400ebc Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Tue, 9 May 2023 15:57:49 +0000 Subject: [PATCH 19/27] fixed linting --- tests/conftest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 8a14ac359..997c8359a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,13 +1,13 @@ # Based on https://docs.pytest.org/en/latest/example/simple.html#control-skipping-of-tests-according-to-command-line-option # noqa: E501 -from mock import Mock import pytest +from fastapi.testclient import TestClient +from mock import Mock from blueapi.core.context import BlueskyContext -from blueapi.worker.reworker import RunEngineWorker from blueapi.service.handler import Handler, get_handler from blueapi.service.main import app -from fastapi.testclient import TestClient +from blueapi.worker.reworker import RunEngineWorker def pytest_addoption(parser): From a2731f38bc84866ae265610eeaf696116c386e72 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Tue, 9 May 2023 16:06:58 +0000 Subject: [PATCH 20/27] made minor changes in response to comments; changed dependencies --- pyproject.toml | 2 +- src/blueapi/cli/cli.py | 1 - tests/service/test_rest_api.py | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 877760438..d2b4e7028 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,7 +24,7 @@ dependencies = [ "click", "fastapi", "uvicorn", - "httpx", + "requests", ] dynamic = ["version"] license.file = "LICENSE" diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index c8c83fa44..c7206770d 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -106,4 +106,3 @@ def run_plan(obj: dict, name: str, parameters: str) -> None: json={"name": name, "params": json.loads(parameters)}, ) print(f"Response returned with {resp.status_code}: ") - pprint(resp.json()) diff --git a/tests/service/test_rest_api.py b/tests/service/test_rest_api.py index a488198e8..5035c51f4 100644 --- a/tests/service/test_rest_api.py +++ b/tests/service/test_rest_api.py @@ -80,7 +80,7 @@ def test_put_plan_submits_task(handler: Handler, client: TestClient) -> None: def on_submit(name: str, task: Task): submitted_tasks[name] = task - handler.worker.submit_task.side_effect = on_submit + handler.worker.submit_task.side_effect = on_submit # type: ignore client.put(f"/task/{task_name}", json=task_json) assert submitted_tasks == {task_name: RunPlan(name=task_name, params=task_json)} From 486cc6c30d0eecab2c52cec8315e98e7f9025338 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Tue, 9 May 2023 16:14:56 +0000 Subject: [PATCH 21/27] modified pyproject toml to include all fastapi dependencies --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index d2b4e7028..3f00faea0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,7 +22,7 @@ dependencies = [ "scanspec", "PyYAML", "click", - "fastapi", + "fastapi[all]", "uvicorn", "requests", ] From 6c0cce437469f7ece794280338789abdc6536dc4 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Thu, 11 May 2023 08:52:02 +0000 Subject: [PATCH 22/27] removed redundant fake_cli file which I just used for testing. Changed error message for deprecated worker command --- src/blueapi/cli/cli.py | 2 +- src/blueapi/cli/fake_cli.py | 38 ------------------------------------- 2 files changed, 1 insertion(+), 39 deletions(-) delete mode 100644 src/blueapi/cli/fake_cli.py diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index c7206770d..8ab7f564e 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -44,7 +44,7 @@ def start_application(obj: dict): @main.command(name="worker", deprecated=True) @click.pass_obj def deprecated_start_application(obj: dict): - print("Please use run command instead.\n") + print("Please use serve command instead.\n") start(obj["config"]) diff --git a/src/blueapi/cli/fake_cli.py b/src/blueapi/cli/fake_cli.py deleted file mode 100644 index 129fe547b..000000000 --- a/src/blueapi/cli/fake_cli.py +++ /dev/null @@ -1,38 +0,0 @@ -import os - -import click - - -class Repo(object): - def __init__(self, home=None, debug=False): - self.home = os.path.abspath(home or ".") - self.debug = debug - - -@click.group() -@click.option("--repo-home", envvar="REPO_HOME", default=".repo") -@click.option("--debug/--no-debug", default=False, envvar="REPO_DEBUG") -@click.pass_context -def cli(ctx, repo_home, debug): - ctx.obj = Repo(repo_home, debug) - - -pass_repo = click.make_pass_decorator(Repo) - - -@cli.command() -@click.argument("src") -@click.argument("dest", required=False) -@pass_repo -def clone(repo, src, dest): - pass - - -@cli.command() -@pass_repo -def cp(repo): - click.echo(isinstance(repo, Repo)) - - -if __name__ == "__main__": - cli() From 5cbba67991ed5a9d7e0d85e12084d23ccb742050 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Thu, 11 May 2023 08:55:34 +0000 Subject: [PATCH 23/27] fixing tests --- tests/test_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index d6b0a8f0c..86daff3ac 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -69,7 +69,7 @@ def test_deprecated_worker_command( assert result.output == ( "DeprecationWarning: The command 'worker' is deprecated.\n" - + "Please use run command instead.\n\n" + + "Please use serve command instead.\n\n" ) From 75c6cdd2283c4196332355896dcedc0c84018f3e Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Thu, 11 May 2023 09:34:58 +0000 Subject: [PATCH 24/27] added extra test for the handler --- src/blueapi/service/handler.py | 2 ++ tests/conftest.py | 3 +++ tests/service/test_handler.py | 25 +++++++++++++++++++++++++ tests/test_cli.py | 8 +++++++- 4 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 tests/service/test_handler.py diff --git a/src/blueapi/service/handler.py b/src/blueapi/service/handler.py index 438d93aa6..f99a7aee6 100644 --- a/src/blueapi/service/handler.py +++ b/src/blueapi/service/handler.py @@ -69,7 +69,9 @@ def get_handler() -> Handler: def teardown_handler() -> None: """Stop all handler tasks. Does nothing if setup_handler has not been called.""" + global HANDLER if HANDLER is None: return handler = get_handler() handler.stop() + HANDLER = None diff --git a/tests/conftest.py b/tests/conftest.py index 997c8359a..1174e7859 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -42,6 +42,9 @@ def __init__(self) -> None: def start(self): return None + def stop(self): + return None + class Client: def __init__(self, handler: MockHandler) -> None: diff --git a/tests/service/test_handler.py b/tests/service/test_handler.py new file mode 100644 index 000000000..acb51b999 --- /dev/null +++ b/tests/service/test_handler.py @@ -0,0 +1,25 @@ +import pytest +from mock import Mock, patch + +from blueapi.service.handler import ( + Handler, + get_handler, + setup_handler, + teardown_handler, +) + + +@patch("blueapi.service.handler.Handler") +def test_get_handler_raises_before_setup_hadler_called( + mock_handler: Mock, handler: Handler +): + mock_handler.side_effect = Mock(return_value=handler) + + with pytest.raises(ValueError): + handler = get_handler() + + setup_handler() + handler = get_handler() + assert handler + + teardown_handler() diff --git a/tests/test_cli.py b/tests/test_cli.py index 86daff3ac..b07b05d33 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -9,7 +9,7 @@ from blueapi import __version__ from blueapi.cli.cli import main from blueapi.core.bluesky_types import Plan -from blueapi.service.handler import Handler +from blueapi.service.handler import Handler, teardown_handler @pytest.fixture @@ -127,6 +127,9 @@ def test_get_plans_and_devices( + "\n{'devices': [{'name': 'my-device', 'protocols': ['HasName']}]}\n" ) + # manually teardown handler, as normally uvicorn does this. + teardown_handler() + def test_invalid_config_path_handling(runner: CliRunner): # test what happens if you pass an invalid config file... @@ -160,3 +163,6 @@ def test_config_passed_down_to_command_children( assert mock_requests.call_args[1] == { "json": {"name": "sleep", "params": {"time": 5}} } + + # manually teardown handler, as normally uvicorn does this. + teardown_handler() From 8a633c2dcdae6afee5ba99825604b3faf2fd8731 Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Thu, 11 May 2023 09:44:51 +0000 Subject: [PATCH 25/27] made suggested changes --- src/blueapi/service/main.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index db38af02b..bb4f64902 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -6,13 +6,14 @@ from blueapi.worker import RunPlan from .handler import Handler, get_handler, setup_handler, teardown_handler -from .model import DeviceModel, DeviceResponse, PlanModel, PlanResponse +from .model import DeviceModel, DeviceResponse, PlanModel, PlanResponse, TaskResponse app = FastAPI(docs_url="/docs", on_shutdown=[teardown_handler]) @app.get("/plans", response_model=PlanResponse) async def get_plans(handler: Handler = Depends(get_handler)): + """Retrieve information about all available plans.""" return PlanResponse( plans=[PlanModel.from_plan(plan) for plan in handler.context.plans.values()] ) @@ -20,6 +21,7 @@ async def get_plans(handler: Handler = Depends(get_handler)): @app.get("/plan/{name}", response_model=PlanModel) async def get_plan_by_name(name: str, handler: Handler = Depends(get_handler)): + """Retrieve information about a plan by its (unique) name.""" try: return PlanModel.from_plan(handler.context.plans[name]) except KeyError: @@ -28,6 +30,7 @@ async def get_plan_by_name(name: str, handler: Handler = Depends(get_handler)): @app.get("/devices", response_model=DeviceResponse) async def get_devices(handler: Handler = Depends(get_handler)): + """Retrieve information about all available devices.""" return DeviceResponse( devices=[ DeviceModel.from_device(device) @@ -38,19 +41,22 @@ async def get_devices(handler: Handler = Depends(get_handler)): @app.get("/device/{name}", response_model=DeviceModel) async def get_device_by_name(name: str, handler: Handler = Depends(get_handler)): + """Retrieve information about a devices by its (unique) name.""" try: return DeviceModel.from_device(handler.context.devices[name]) except KeyError: raise HTTPException(status_code=404, detail="Item not found") -@app.put("/task/{name}") -async def execute_task( +@app.put("/task/{name}", response_model=TaskResponse) +async def submit_task( name: str, task: Mapping[str, Any] = Body(..., example={"detectors": ["x"]}), handler: Handler = Depends(get_handler), ): + """Submit a task onto the worker queue.""" handler.worker.submit_task(name, RunPlan(name=name, params=task)) + return TaskResponse(task_name=f"Task {name} submitted") def start(config: ApplicationConfig): From a724fab1f9f2c0f6e72bd279bc77bc197d8718db Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova <90774497+RAYemelyanova@users.noreply.github.com> Date: Thu, 11 May 2023 10:50:23 +0100 Subject: [PATCH 26/27] Update src/blueapi/service/main.py Co-authored-by: Callum Forrester <29771545+callumforrester@users.noreply.github.com> --- src/blueapi/service/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index bb4f64902..60b0a6114 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -56,7 +56,7 @@ async def submit_task( ): """Submit a task onto the worker queue.""" handler.worker.submit_task(name, RunPlan(name=name, params=task)) - return TaskResponse(task_name=f"Task {name} submitted") + return TaskResponse(task_name=name) def start(config: ApplicationConfig): From 3863f94a4d398ae02aebfb0382a2eadb228672ca Mon Sep 17 00:00:00 2001 From: Rose Yemelyanova Date: Thu, 11 May 2023 10:19:12 +0000 Subject: [PATCH 27/27] modified docs --- docs/developer/tutorials/dev-run.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer/tutorials/dev-run.rst b/docs/developer/tutorials/dev-run.rst index fd508b260..cd32033ea 100644 --- a/docs/developer/tutorials/dev-run.rst +++ b/docs/developer/tutorials/dev-run.rst @@ -44,7 +44,7 @@ Start the worker from the command line or vscode: .. code:: shell - blueapi worker + blueapi serve .. tab-item:: VSCode