Skip to content

Commit

Permalink
Make environment error message be a non-empty string or None (#571)
Browse files Browse the repository at this point in the history
The code was using `None` or `""` in various places to denote the lack
of an error message. `None` is a better semantic fit, so this PR forces
the user to pass either `None` or a string with at least 1 character.
  • Loading branch information
callumforrester authored Jul 26, 2024
1 parent 3461bcc commit 06988d1
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 16 deletions.
1 change: 1 addition & 0 deletions docs/reference/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ components:
properties:
error_message:
description: If present - error loading context
minLength: 1
title: Error Message
type: string
initialized:
Expand Down
4 changes: 3 additions & 1 deletion src/blueapi/service/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,5 +145,7 @@ class EnvironmentResponse(BlueapiBaseModel):

initialized: bool = Field(description="blueapi context initialized")
error_message: str | None = Field(
default=None, description="If present - error loading context"
default=None,
description="If present - error loading context",
min_length=1,
)
12 changes: 5 additions & 7 deletions src/blueapi/service/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@

from blueapi.config import ApplicationConfig
from blueapi.service.interface import InitialisationException, start_worker, stop_worker
from blueapi.service.model import (
EnvironmentResponse,
)
from blueapi.service.model import EnvironmentResponse

# The default multiprocessing start method is fork
set_start_method("spawn", force=True)
Expand Down Expand Up @@ -39,7 +37,7 @@ def __init__(
self._config = config or ApplicationConfig()
self._subprocess = None
self._use_subprocess = use_subprocess
self._state = EnvironmentResponse(initialized=False, error_message="")
self._state = EnvironmentResponse(initialized=False)

def start(self):
if self._subprocess is None and self._use_subprocess:
Expand All @@ -55,11 +53,11 @@ def start(self):
)
LOGGER.exception(self._state.error_message)
return
self._state = EnvironmentResponse(initialized=True, error_message="")
self._state = EnvironmentResponse(initialized=True)

def stop(self):
if self._subprocess is not None:
self._state = EnvironmentResponse(initialized=False, error_message="")
self._state = EnvironmentResponse(initialized=False)
try:
self._subprocess.apply(stop_worker)
except InitialisationException:
Expand All @@ -71,7 +69,7 @@ def stop(self):
if (not self._use_subprocess) and (
self._state.initialized or self._state.error_message
):
self._state = EnvironmentResponse(initialized=False, error_message="")
self._state = EnvironmentResponse(initialized=False)
stop_worker()

def reload_context(self):
Expand Down
9 changes: 2 additions & 7 deletions tests/service/test_rest_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,7 @@

from blueapi.core.bluesky_types import Plan
from blueapi.service import main
from blueapi.service.model import (
DeviceModel,
PlanModel,
StateChangeRequest,
WorkerTask,
)
from blueapi.service.model import DeviceModel, PlanModel, StateChangeRequest, WorkerTask
from blueapi.worker.event import WorkerState
from blueapi.worker.task import Task
from blueapi.worker.worker import TrackableTask
Expand Down Expand Up @@ -546,7 +541,7 @@ def test_set_state_invalid_transition(
def test_get_environment_idle(client: TestClient) -> None:
assert client.get("/environment").json() == {
"initialized": True,
"error_message": "",
"error_message": None,
}


Expand Down
2 changes: 1 addition & 1 deletion tests/service/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,4 @@ def test_can_reload_after_an_error(pool_mock: MagicMock):

runner.reload_context()

assert runner.state == EnvironmentResponse(initialized=True, error_message="")
assert runner.state == EnvironmentResponse(initialized=True, error_message=None)

0 comments on commit 06988d1

Please sign in to comment.