Skip to content

Commit

Permalink
Fix reloading the environment if there is an error
Browse files Browse the repository at this point in the history
This adds a new error field to the environment. This is now checked
against when attempting to delete the environment as if there is an
error the handler is never set to initialized.

Note that the property has only been added to SubprocessHandler as
the BlueskyHandler ABC is likely to be removed.

Fixes #512.
  • Loading branch information
joeshannon committed Jun 20, 2024
1 parent 31c94c5 commit aa88466
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 14 deletions.
9 changes: 9 additions & 0 deletions docs/reference/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,21 @@ components:
additionalProperties: false
description: State of internal environment.
properties:
error:
description: Error starting blueapi context
title: Error
type: boolean
error_message:
description: Contains error message if environment could not be initalized
title: Error Message
type: string
initialized:
description: blueapi context initialized
title: Initialized
type: boolean
required:
- initialized
- error
title: EnvironmentResponse
type: object
HTTPValidationError:
Expand Down
13 changes: 10 additions & 3 deletions src/blueapi/service/handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@
from blueapi.messaging import StompMessagingTemplate
from blueapi.messaging.base import MessagingTemplate
from blueapi.service.handler_base import BlueskyHandler
from blueapi.service.model import DeviceModel, PlanModel, WorkerTask
from blueapi.service.model import (
DeviceModel,
EnvironmentResponse,
PlanModel,
WorkerTask,
)
from blueapi.worker.event import TaskStatusEnum, WorkerState
from blueapi.worker.reworker import TaskWorker
from blueapi.worker.task import Task
Expand Down Expand Up @@ -143,8 +148,10 @@ def get_task_by_id(self, task_id: str) -> TrackableTask | None:
return self._worker.get_task_by_id(task_id)

@property
def initialized(self) -> bool:
return self._initialized
def initialized(self) -> EnvironmentResponse:
return EnvironmentResponse(
initialized=self._initialized, error=False, error_message=None
)


HANDLER: Handler | None = None
Expand Down
9 changes: 7 additions & 2 deletions src/blueapi/service/handler_base.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
from abc import ABC, abstractmethod

from blueapi.service.model import DeviceModel, PlanModel, WorkerTask
from blueapi.service.model import (
DeviceModel,
EnvironmentResponse,
PlanModel,
WorkerTask,
)
from blueapi.worker.event import TaskStatusEnum, WorkerState
from blueapi.worker.task import Task
from blueapi.worker.worker import TrackableTask
Expand Down Expand Up @@ -103,7 +108,7 @@ def stop(self):

@property
@abstractmethod
def initialized(self) -> bool:
def initialized(self) -> EnvironmentResponse:
"""Handler initialization state"""


Expand Down
4 changes: 2 additions & 2 deletions src/blueapi/service/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ async def on_key_error_404(_: Request, __: KeyError):
def get_environment(
handler: BlueskyHandler = Depends(get_handler),
) -> EnvironmentResponse:
return EnvironmentResponse(initialized=handler.initialized)
return handler.initialized


@app.delete("/environment")
Expand All @@ -100,7 +100,7 @@ def restart_handler(handler: BlueskyHandler):
handler.stop()
handler.start()

if handler.initialized:
if handler.initialized.initialized or handler.initialized.error:
background_tasks.add_task(restart_handler, handler)


Expand Down
4 changes: 4 additions & 0 deletions src/blueapi/service/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,7 @@ class EnvironmentResponse(BlueapiBaseModel):
"""

initialized: bool = Field(description="blueapi context initialized")
error: bool = Field(description="Error starting blueapi context")
error_message: str | None = Field(
description="Contains error message if environment could not be initalized"
)
26 changes: 22 additions & 4 deletions src/blueapi/service/subprocess_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@
from blueapi.config import ApplicationConfig
from blueapi.service.handler import get_handler, setup_handler, teardown_handler
from blueapi.service.handler_base import BlueskyHandler, HandlerNotStartedError
from blueapi.service.model import DeviceModel, PlanModel, WorkerTask
from blueapi.service.model import (
DeviceModel,
EnvironmentResponse,
PlanModel,
WorkerTask,
)
from blueapi.worker.event import TaskStatusEnum, WorkerState
from blueapi.worker.task import Task
from blueapi.worker.worker import TrackableTask
Expand All @@ -25,6 +30,8 @@ class SubprocessHandler(BlueskyHandler):
_config: ApplicationConfig
_subprocess: PoolClass | None
_initialized: bool = False
_error: bool = False
_error_message: str | None = None

def __init__(
self,
Expand All @@ -39,7 +46,12 @@ def start(self):
self._subprocess.apply(
logging.basicConfig, kwds={"level": self._config.logging.level}
)
self._subprocess.apply(setup_handler, [self._config])
try:
self._subprocess.apply(setup_handler, [self._config])
except Exception as e:
self._error = True
self._error_message = f"Error starting blueapi: {e}"
return
self._initialized = True

def stop(self):
Expand All @@ -48,6 +60,8 @@ def stop(self):
self._subprocess.apply(teardown_handler)
self._subprocess.close()
self._subprocess.join()
self._error = False
self._error_message = None
self._subprocess = None

def reload_context(self):
Expand Down Expand Up @@ -113,8 +127,12 @@ def get_task_by_id(self, task_id: str) -> TrackableTask | None:
return self._run_in_subprocess(get_task_by_id, [task_id])

@property
def initialized(self) -> bool:
return self._initialized
def initialized(self) -> EnvironmentResponse:
return EnvironmentResponse(
initialized=self._initialized,
error=self._error,
error_message=self._error_message,
)


# Free functions (passed to subprocess) for each of the methods required by Handler
Expand Down
2 changes: 1 addition & 1 deletion tests/service/test_rest_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ def test_current_complete_returns_400(


def test_get_environment(handler: Handler, client: TestClient) -> None:
assert client.get("/environment").json() == {"initialized": False}
assert client.get("/environment").json() == {"initialized": False, "error": False}


def test_delete_environment(handler: Handler, client: TestClient) -> None:
Expand Down
9 changes: 7 additions & 2 deletions tests/service/test_subprocess_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
import pytest

from blueapi.service.handler_base import BlueskyHandler, HandlerNotStartedError
from blueapi.service.model import DeviceModel, PlanModel, WorkerTask
from blueapi.service.model import (
DeviceModel,
EnvironmentResponse,
PlanModel,
WorkerTask,
)
from blueapi.service.subprocess_handler import SubprocessHandler
from blueapi.worker.event import TaskStatusEnum, WorkerState
from blueapi.worker.task import Task
Expand Down Expand Up @@ -128,7 +133,7 @@ def stop(self): ...
# Initialized is a special case as it is not delegated
# Tested by test_initialize
@property
def initialized(self) -> bool:
def initialized(self) -> EnvironmentResponse:
raise Exception("Not implemented")


Expand Down

0 comments on commit aa88466

Please sign in to comment.