Revert stomp config changes

Author: tpoliaw

docs/reference/openapi.yaml

@@ -1,22 +1,5 @@
 components:
   schemas:
-    BasicAuthentication:
-      additionalProperties: false
-      description: User credentials for basic authentication
-      properties:
-        password:
-          description: Password to verify user's identity
-          title: Password
-          type: string
-        username:
-          description: Unique identifier for user
-          title: Username
-          type: string
-      required:
-      - username
-      - password
-      title: BasicAuthentication
-      type: object
     DeviceModel:
       additionalProperties: false
       description: Representation of a device
@@ -241,28 +224,6 @@ components:
       - new_state
       title: StateChangeRequest
       type: object
-    StompConfig:
-      additionalProperties: false
-      description: Config for connecting to stomp broker
-      properties:
-        auth:
-          $ref: '#/components/schemas/BasicAuthentication'
-          description: Auth information for communicating with STOMP broker, if required
-          title: Auth
-        enabled:
-          default: false
-          description: True if blueapi should connect to stomp for asynchronous event
-            publishing
-          title: Enabled
-          type: boolean
-        url:
-          default: tcp://localhost:61613
-          format: uri
-          minLength: 1
-          title: Url
-          type: string
-      title: StompConfig
-      type: object
     Task:
       additionalProperties: false
       description: Task that will run a plan
@@ -472,7 +433,7 @@ info:
     name: Apache 2.0
     url: https://www.apache.org/licenses/LICENSE-2.0.html
   title: BlueAPI Control
-  version: 1.3.0
+  version: 1.2.0
 openapi: 3.1.0
 paths:
   /config/oidc:
@@ -491,22 +452,6 @@ paths:
       summary: Get Oidc Config
       tags:
       - Meta
-  /config/stomp:
-    get:
-      description: Retrieve the stomp configuration for the server.
-      operationId: get_stomp_config_config_stomp_get
-      responses:
-        '200':
-          content:
-            application/json:
-              schema:
-                $ref: '#/components/schemas/StompConfig'
-          description: Successful Response
-        '204':
-          description: No Stomp configured
-      summary: Get Stomp Config
-      tags:
-      - Meta
   /devices:
     get:
       description: Retrieve information about all available devices.

helm/blueapi/config_schema.json

@@ -480,10 +480,16 @@
                     "type": "string"
                 },
                 "auth": {
-                    "$ref": "BasicAuthentication",
+                    "anyOf": [
+                        {
+                            "$ref": "BasicAuthentication"
+                        },
+                        {
+                            "type": "null"
+                        }
+                    ],
                     "default": null,
-                    "description": "Auth information for communicating with STOMP broker, if required",
-                    "title": "Auth"
+                    "description": "Auth information for communicating with STOMP broker, if required"
                 }
             },
             "title": "StompConfig",

helm/blueapi/values.schema.json

@@ -901,9 +901,15 @@
             "type": "object",
             "properties": {
                 "auth": {
-                    "title": "Auth",
                     "description": "Auth information for communicating with STOMP broker, if required",
-                    "$ref": "BasicAuthentication"
+                    "anyOf": [
+                        {
+                            "$ref": "BasicAuthentication"
+                        },
+                        {
+                            "type": "null"
+                        }
+                    ]
                 },
                 "enabled": {
                     "title": "Enabled",

src/blueapi/client/client.py

@@ -8,8 +8,12 @@
 from pathlib import Path
 from typing import Self
 
-from bluesky_stomp.messaging import MessageContext
-from observability_utils.tracing import get_tracer, start_as_current_span
+from bluesky_stomp.messaging import MessageContext, StompClient
+from bluesky_stomp.models import Broker
+from observability_utils.tracing import (
+    get_tracer,
+    start_as_current_span,
+)
 
 from blueapi.config import (
     ApplicationConfig,
@@ -197,7 +201,7 @@ class BlueapiClient:
     """Unified client for controlling blueapi"""
 
     _rest: BlueapiRestClient
-    _event_bus_client: EventBusClient | None
+    _events: EventBusClient | None
     _instrument_session: str | None = None
     _callbacks: dict[int, OnAnyEvent]
     _callback_id: itertools.count
@@ -208,7 +212,7 @@ def __init__(
         events: EventBusClient | None = None,
     ):
         self._rest = rest
-        self._event_bus_client = events
+        self._events = events
         self._callbacks = {}
         self._callback_id = itertools.count()
 
@@ -226,8 +230,20 @@ def from_config(cls, config: ApplicationConfig) -> Self:
         except Exception:
             ...  # Swallow exceptions
         rest = BlueapiRestClient(config.api, session_manager=session_manager)
-        event_bus = EventBusClient.from_stomp_config(config.stomp)
-        return cls(rest, event_bus)
+        if config.stomp.enabled:
+            assert config.stomp.url.host is not None, "Stomp URL missing host"
+            assert config.stomp.url.port is not None, "Stomp URL missing port"
+            client = StompClient.for_broker(
+                broker=Broker(
+                    host=config.stomp.url.host,
+                    port=config.stomp.url.port,
+                    auth=config.stomp.auth,
+                )
+            )
+            events = EventBusClient(client)
+            return cls(rest, events)
+        else:
+            return cls(rest)
 
     @cached_property
     @start_as_current_span(TRACER)
@@ -444,7 +460,7 @@ def run_task(
             of task execution.
         """
 
-        if (event_bus := self._event_bus()) is None:
+        if self._events is None:
             raise MissingStompConfigurationError(
                 "Stomp configuration required to run plans is missing or disabled"
             )
@@ -488,8 +504,8 @@ def inner_on_event(event: AnyEvent, ctx: MessageContext) -> None:
                     else:
                         complete.set_result(event.task_status)
 
-        with event_bus:
-            event_bus.subscribe_to_all_events(inner_on_event)
+        with self._events:
+            self._events.subscribe_to_all_events(inner_on_event)
             self._rest.update_worker_task(WorkerTask(task_id=task_id))
             return complete.result(timeout=timeout)
 
@@ -728,10 +744,3 @@ def login(self, token_path: Path | None = None):
                 auth.start_device_flow()
             else:
                 print("Server is not configured to use authentication!")
-
-    def _event_bus(self) -> EventBusClient | None:
-        if not self._event_bus_client:
-            if stomp_config := self._rest.get_stomp_config():
-                self._event_bus_client = EventBusClient.from_stomp_config(stomp_config)
-
-        return self._event_bus_client

src/blueapi/client/event_bus.py

@@ -1,10 +1,8 @@
 from collections.abc import Callable
-from typing import Self
 
-from bluesky_stomp.messaging import Broker, MessageContext, StompClient
+from bluesky_stomp.messaging import MessageContext, StompClient
 from bluesky_stomp.models import MessageTopic
 
-from blueapi.config import StompConfig
 from blueapi.core import DataEvent
 from blueapi.worker import ProgressEvent, WorkerEvent
 
@@ -47,15 +45,3 @@ def subscribe_to_all_events(
             raise BlueskyStreamingError(
                 "Unable to subscribe to messages from blueapi"
             ) from err
-
-    @classmethod
-    def from_stomp_config(cls, config: StompConfig) -> Self | None:
-        if config.enabled:
-            assert config.url.host is not None, "Stomp URL missing host"
-            assert config.url.port is not None, "Stomp URL missing port"
-            client = StompClient.for_broker(
-                broker=Broker(
-                    host=config.url.host, port=config.url.port, auth=config.auth
-                )
-            )
-            return cls(client)

src/blueapi/client/rest.py

@@ -10,7 +10,7 @@
 )
 from pydantic import BaseModel, TypeAdapter, ValidationError
 
-from blueapi.config import RestConfig, StompConfig
+from blueapi.config import RestConfig
 from blueapi.service.authentication import JWTAuth, SessionManager
 from blueapi.service.model import (
     DeviceModel,
@@ -233,14 +233,6 @@ def get_oidc_config(self) -> OIDCConfig | None:
             # Server is not using authentication
             return None
 
-    def get_stomp_config(self) -> StompConfig | None:
-        try:
-            return self._request_and_deserialize("/config/stomp", StompConfig)
-        except (NoContentError, KeyError):
-            # Older versions of the server may not have the endpoint implemented so
-            # treat 404s as no configuration.
-            return None
-
     def get_python_environment(
         self, name: str | None = None, source: SourceInfo | None = None
     ) -> PythonEnvironmentResponse:

src/blueapi/config.py

@@ -102,7 +102,7 @@ class StompConfig(BlueapiBaseModel):
         default=False,
     )
     url: TcpUrl = TcpUrl("tcp://localhost:61613")
-    auth: BasicAuthentication | SkipJsonSchema[None] = Field(
+    auth: BasicAuthentication | None = Field(
         description="Auth information for communicating with STOMP broker, if required",
         default=None,
     )
@@ -300,7 +300,7 @@ class ApplicationConfig(BlueapiBaseModel):
     """
 
     #: API version to publish in OpenAPI schema
-    REST_API_VERSION: ClassVar[str] = "1.3.0"
+    REST_API_VERSION: ClassVar[str] = "1.2.0"
 
     LICENSE_INFO: ClassVar[dict[str, str]] = {
         "name": "Apache 2.0",

src/blueapi/service/interface.py

@@ -272,10 +272,6 @@ def get_oidc_config() -> OIDCConfig | None:
     return config().oidc
 
 
-def get_stomp_config() -> StompConfig | None:
-    return config().stomp
-
-
 def get_python_env(
     name: str | None = None, source: SourceInfo | None = None
 ) -> PythonEnvironmentResponse:

src/blueapi/service/main.py

@@ -34,7 +34,7 @@
 from starlette.responses import JSONResponse
 from super_state_machine.errors import TransitionError
 
-from blueapi.config import ApplicationConfig, OIDCConfig, StompConfig, Tag
+from blueapi.config import ApplicationConfig, OIDCConfig, Tag
 from blueapi.service import interface
 from blueapi.worker import TrackableTask, WorkerState
 from blueapi.worker.event import TaskStatusEnum
@@ -226,22 +226,6 @@ def get_oidc_config(
     return config
 
 
-@open_router.get(
-    "/config/stomp",
-    tags=[Tag.META],
-    responses={status.HTTP_204_NO_CONTENT: {"description": "No Stomp configured"}},
-)
-@start_as_current_span(TRACER)
-def get_stomp_config(
-    runner: Annotated[WorkerDispatcher, Depends(_runner)],
-) -> StompConfig:
-    """Retrieve the stomp configuration for the server."""
-    config = runner.run(interface.get_stomp_config)
-    if config is None:
-        raise HTTPException(status_code=status.HTTP_204_NO_CONTENT)
-    return config
-
-
 @secure_router.get("/plans", tags=[Tag.PLAN])
 @start_as_current_span(TRACER)
 def get_plans(runner: Annotated[WorkerDispatcher, Depends(_runner)]) -> PlanResponse:

tests/system_tests/test_blueapi_system.py

@@ -169,21 +169,16 @@ def expected_devices() -> DeviceResponse:
     )
 
 
-def authenticated_get_methods() -> list[str]:
-    # Get a list of methods that take only one argument (self) and require
-    # authentication. This will currently return
-    # ['get_plans', 'get_devices', 'get_state', 'get_all_tasks',
-    # 'get_active_task','get_environment','resume', 'stop']
+@pytest.fixture
+def blueapi_rest_client_get_methods() -> list[str]:
+    # Get a list of methods that take only one argument (self)
     return [
-        method
-        for method in BlueapiRestClient.__dict__
-        if callable(getattr(BlueapiRestClient, method))
-        and not method.startswith("__")
-        and len(inspect.signature(getattr(BlueapiRestClient, method)).parameters) == 1
-        and "self" in inspect.signature(getattr(BlueapiRestClient, method)).parameters
-        # oidc_config and stomp config can be accessed without auth
-        and method != "get_oidc_config"
-        and method != "get_stomp_config"
+        name
+        for name, method in BlueapiRestClient.__dict__.items()
+        if not name.startswith("__")
+        and callable(method)
+        and len(params := inspect.signature(method).parameters) == 1
+        and "self" in params
     ]
 
 
@@ -215,10 +210,15 @@ def reset_numtracker():
     yield
 
 
-@pytest.mark.parametrize("method_name", authenticated_get_methods())
-def test_cannot_access_endpoints(client_without_auth: BlueapiClient, method_name: str):
-    with pytest.raises(BlueskyRemoteControlError, match=r"<Response \[401\]>"):
-        getattr(client_without_auth._rest, method_name)()
+def test_cannot_access_endpoints(
+    client_without_auth: BlueapiClient, blueapi_rest_client_get_methods: list[str]
+):
+    blueapi_rest_client_get_methods.remove(
+        "get_oidc_config"
+    )  # get_oidc_config can be accessed without auth
+    for get_method in blueapi_rest_client_get_methods:
+        with pytest.raises(BlueskyRemoteControlError, match=r"<Response \[401\]>"):
+            getattr(client_without_auth._rest, get_method)()
 
 
 def test_can_get_oidc_config_without_auth(client_without_auth: BlueapiClient):