Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove RunPlan class that wraps Task #382

Merged
merged 5 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
203 changes: 102 additions & 101 deletions docs/user/reference/openapi.yaml
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you generate this file? It'd be good to remove all the unrelated formatting only changes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I just ran ruff / pre-commit automatically

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now I auto-generated a new one using
python -m blueapi.cli schema -u.
formatting indeed has changed

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/blueapi/cli/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
print_schema_as_yaml,
write_schema_as_yaml,
)
from blueapi.worker import ProgressEvent, RunPlan, WorkerEvent, WorkerState
from blueapi.worker import ProgressEvent, Task, WorkerEvent, WorkerState

from .rest import BlueapiRestClient

Expand Down Expand Up @@ -182,7 +182,7 @@ def store_finished_event(event: WorkerEvent) -> None:
finished_event.append(event)

parameters = parameters or "{}"
task = RunPlan(name=name, params=json.loads(parameters))
task = Task(name=name, params=json.loads(parameters))

resp = client.create_task(task)
task_id = resp.task_id
Expand Down
10 changes: 4 additions & 6 deletions src/blueapi/cli/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
TaskResponse,
WorkerTask,
)
from blueapi.worker import RunPlan, TrackableTask, WorkerState
from blueapi.worker import Task, TrackableTask, WorkerState

from .amq import BlueskyRemoteError

Expand Down Expand Up @@ -56,15 +56,13 @@
data={"new_state": state, "defer": defer},
)

def get_task(self, task_id: str) -> TrackableTask[RunPlan]:
return self._request_and_deserialize(
f"/tasks/{task_id}", TrackableTask[RunPlan]
)
def get_task(self, task_id: str) -> TrackableTask[Task]:
return self._request_and_deserialize(f"/tasks/{task_id}", TrackableTask[Task])

Check warning on line 60 in src/blueapi/cli/rest.py

View check run for this annotation

Codecov / codecov/patch

src/blueapi/cli/rest.py#L60

Added line #L60 was not covered by tests

def get_active_task(self) -> WorkerTask:
return self._request_and_deserialize("/worker/task", WorkerTask)

def create_task(self, task: RunPlan) -> TaskResponse:
def create_task(self, task: Task) -> TaskResponse:
return self._request_and_deserialize(
"/tasks",
TaskResponse,
Expand Down
4 changes: 2 additions & 2 deletions src/blueapi/service/handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from blueapi.service.model import DeviceModel, PlanModel, WorkerTask
from blueapi.worker.event import WorkerState
from blueapi.worker.reworker import RunEngineWorker
from blueapi.worker.task import RunPlan
from blueapi.worker.task import Task
from blueapi.worker.worker import TrackableTask, Worker

LOGGER = logging.getLogger(__name__)
Expand Down Expand Up @@ -102,7 +102,7 @@ def devices(self) -> List[DeviceModel]:
def get_device(self, name: str) -> DeviceModel:
return DeviceModel.from_device(self._context.devices[name])

def submit_task(self, task: RunPlan) -> str:
def submit_task(self, task: Task) -> str:
return self._worker.submit_task(task)

def clear_pending_task(self, task_id: str) -> str:
Expand Down
4 changes: 2 additions & 2 deletions src/blueapi/service/handler_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

from blueapi.service.model import DeviceModel, PlanModel, WorkerTask
from blueapi.worker.event import WorkerState
from blueapi.worker.task import RunPlan
from blueapi.worker.task import Task
from blueapi.worker.worker import TrackableTask


Expand Down Expand Up @@ -37,7 +37,7 @@ def get_device(self, name: str) -> DeviceModel:
"""

@abstractmethod
def submit_task(self, task: RunPlan) -> str:
def submit_task(self, task: Task) -> str:
"""
Submit a task to be run on begin_task
"""
Expand Down
6 changes: 2 additions & 4 deletions src/blueapi/service/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from super_state_machine.errors import TransitionError

from blueapi.config import ApplicationConfig
from blueapi.worker import RunPlan, TrackableTask, WorkerState
from blueapi.worker import Task, TrackableTask, WorkerState

from .handler_base import BlueskyHandler
from .model import (
Expand Down Expand Up @@ -141,9 +141,7 @@ def get_device_by_name(name: str, handler: BlueskyHandler = Depends(get_handler)
def submit_task(
request: Request,
response: Response,
task: RunPlan = Body(
..., example=RunPlan(name="count", params={"detectors": ["x"]})
),
task: Task = Body(..., example=Task(name="count", params={"detectors": ["x"]})),
handler: BlueskyHandler = Depends(get_handler),
):
"""Submit a task to the worker."""
Expand Down
6 changes: 3 additions & 3 deletions src/blueapi/service/subprocess_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from blueapi.service.handler_base import BlueskyHandler, HandlerNotStartedError
from blueapi.service.model import DeviceModel, PlanModel, WorkerTask
from blueapi.worker.event import WorkerState
from blueapi.worker.task import RunPlan
from blueapi.worker.task import Task
from blueapi.worker.worker import TrackableTask

set_start_method("spawn", force=True)
Expand Down Expand Up @@ -78,7 +78,7 @@ def devices(self) -> List[DeviceModel]:
def get_device(self, name: str) -> DeviceModel:
return self._run_in_subprocess(get_device, [name])

def submit_task(self, task: RunPlan) -> str:
def submit_task(self, task: Task) -> str:
return self._run_in_subprocess(submit_task, [task])

def clear_pending_task(self, task_id: str) -> str:
Expand Down Expand Up @@ -135,7 +135,7 @@ def get_device(name: str) -> DeviceModel:
return get_handler().get_device(name)


def submit_task(task: RunPlan) -> str:
def submit_task(task: Task) -> str:
return get_handler().submit_task(task)


Expand Down
3 changes: 1 addition & 2 deletions src/blueapi/worker/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from .event import ProgressEvent, StatusView, TaskStatus, WorkerEvent, WorkerState
from .multithread import run_worker_in_own_thread
from .reworker import RunEngineWorker
from .task import RunPlan, Task
from .task import Task
from .worker import TrackableTask, Worker
from .worker_busy_error import WorkerBusyError

Expand All @@ -10,7 +10,6 @@
"RunEngineWorker",
"Task",
"Worker",
"RunPlan",
"WorkerEvent",
"WorkerState",
"StatusView",
Expand Down
10 changes: 0 additions & 10 deletions src/blueapi/worker/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,6 @@ def do_task(self, ctx: BlueskyContext) -> None:
ctx.run_engine(wrapped_plan_generator)


# Here for backward compatibility pending
# https://github.com/DiamondLightSource/blueapi/issues/253
class RunPlan(Task):
stan-dot marked this conversation as resolved.
Show resolved Hide resolved
"""
Task that will run a plan
"""

...


def _lookup_params(ctx: BlueskyContext, task: Task) -> BaseModel:
"""
Checks plan parameters against context
Expand Down
4 changes: 2 additions & 2 deletions tests/service/test_rest_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@
from blueapi.service.handler import Handler
from blueapi.service.main import get_handler, setup_handler, teardown_handler
from blueapi.service.model import WorkerTask
from blueapi.worker.task import RunPlan
from blueapi.worker.task import Task
from src.blueapi.worker import WorkerState

_TASK = RunPlan(name="count", params={"detectors": ["x"]})
_TASK = Task(name="count", params={"detectors": ["x"]})


def test_get_plans(handler: Handler, client: TestClient) -> None:
Expand Down
12 changes: 5 additions & 7 deletions tests/service/test_subprocess_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from blueapi.service.model import DeviceModel, PlanModel, WorkerTask
from blueapi.service.subprocess_handler import SubprocessHandler
from blueapi.worker.event import WorkerState
from blueapi.worker.task import RunPlan
from blueapi.worker.task import Task
from blueapi.worker.worker import TrackableTask


Expand Down Expand Up @@ -63,7 +63,7 @@ def devices(self) -> List[DeviceModel]:
def get_device(self, name: str) -> DeviceModel:
return DeviceModel(name="device1", protocols=[])

def submit_task(self, task: RunPlan) -> str:
def submit_task(self, task: Task) -> str:
return "0"

def clear_pending_task(self, task_id: str) -> str:
Expand All @@ -89,9 +89,7 @@ def cancel_active_task(self, failure: bool, reason: Optional[str]) -> None: ...
@property
def pending_tasks(self) -> List[TrackableTask]:
return [
TrackableTask(
task_id="abc", task=RunPlan(name="sleep", params={"time": 0.0})
)
TrackableTask(task_id="abc", task=Task(name="sleep", params={"time": 0.0}))
]

def get_pending_task(self, task_id: str) -> Optional[TrackableTask]:
Expand Down Expand Up @@ -137,8 +135,8 @@ def run_in_same_process(func, args=None):
assert sp_handler.get_device("name") == dummy_handler.get_device("name")

assert sp_handler.submit_task(
RunPlan(name="sleep", params={"time": 0.0})
) == dummy_handler.submit_task(RunPlan(name="sleep", params={"time": 0.0}))
Task(name="sleep", params={"time": 0.0})
) == dummy_handler.submit_task(Task(name="sleep", params={"time": 0.0}))

assert sp_handler.clear_pending_task("task_id") == dummy_handler.clear_pending_task(
"task_id"
Expand Down
15 changes: 6 additions & 9 deletions tests/worker/test_reworker.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from blueapi.worker import (
ProgressEvent,
RunEngineWorker,
RunPlan,
Task,
TaskStatus,
TrackableTask,
Expand All @@ -21,13 +20,13 @@
WorkerState,
)

_SIMPLE_TASK = RunPlan(name="sleep", params={"time": 0.0})
_LONG_TASK = RunPlan(name="sleep", params={"time": 1.0})
_INDEFINITE_TASK = RunPlan(
_SIMPLE_TASK = Task(name="sleep", params={"time": 0.0})
_LONG_TASK = Task(name="sleep", params={"time": 1.0})
_INDEFINITE_TASK = Task(
name="set_absolute",
params={"movable": "fake_device", "value": 4.0},
)
_FAILING_TASK = RunPlan(name="failing_plan", params={})
_FAILING_TASK = Task(name="failing_plan", params={})


class FakeDevice:
Expand Down Expand Up @@ -259,9 +258,7 @@ def test_no_additional_progress_events_after_complete(worker: Worker):
progress_events: List[ProgressEvent] = []
worker.progress_events.subscribe(lambda event, id: progress_events.append(event))

task: Task = RunPlan(
name="move", params={"moves": {"additional_status_device": 5.0}}
)
task: Task = Task(name="move", params={"moves": {"additional_status_device": 5.0}})
task_id = worker.submit_task(task)
begin_task_and_wait_until_complete(worker, task_id)

Expand Down Expand Up @@ -344,7 +341,7 @@ def test_worker_and_data_events_produce_in_order(worker: Worker) -> None:
def assert_running_count_plan_produces_ordered_worker_and_data_events(
expected_events: List[Union[WorkerEvent, DataEvent]],
worker: Worker,
task: Task = RunPlan(name="count", params={"detectors": ["image_det"], "num": 1}),
task: Task = Task(name="count", params={"detectors": ["image_det"], "num": 1}),
timeout: float = 5.0,
) -> None:
event_streams: List[EventStream[Any, int]] = [
Expand Down
Loading