From 8b35390a8a4fd1a39ea573a325b568ce5450dbd4 Mon Sep 17 00:00:00 2001 From: DiamondJoseph <53935796+DiamondJoseph@users.noreply.github.com> Date: Tue, 17 Sep 2024 13:52:47 +0100 Subject: [PATCH] Update bluesky-stomp and prevent duplication of BasicAuthentication (#640) Fixes #636 --- dev-requirements.txt | 58 +++++++------- helm/blueapi/values.yaml | 89 ++++++++++------------ pyproject.toml | 2 +- src/blueapi/cli/cli.py | 4 +- src/blueapi/client/client.py | 4 +- src/blueapi/client/event_bus.py | 6 +- src/blueapi/config.py | 23 +----- src/blueapi/service/interface.py | 6 +- tests/unit_tests/client/test_event_bus.py | 8 +- tests/unit_tests/service/test_interface.py | 10 +-- tests/unit_tests/test_cli.py | 10 +-- tests/unit_tests/test_config.py | 17 +++-- 12 files changed, 107 insertions(+), 130 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index dba27878b..68b3ecac7 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -18,7 +18,7 @@ bidict==0.23.1 bluesky==1.13.0a4 bluesky-kafka==0.10.0 bluesky-live==0.0.8 -bluesky-stomp==0.1.0 +bluesky-stomp==0.1.2 boltons==24.0.0 bump-pydantic==0.8.0 cachetools==5.5.0 @@ -37,14 +37,14 @@ contourpy==1.3.0 copier==9.3.1 coverage==7.6.1 cycler==0.12.1 -dask==2024.8.2 +dask==2024.9.0 databroker==1.2.5 dataclasses-json==0.6.7 decorator==5.1.1 deepmerge==2.0 distlib==0.3.8 dls-bluesky-core==0.0.4 -dls-dodal==1.31.0 +dls-dodal==1.31.1 dnspython==2.6.1 docopt==0.6.2 doct==1.1.0 @@ -56,10 +56,10 @@ epicscorelibs==7.0.7.99.0.2 event-model==1.21.0 exceptiongroup==1.2.2 executing==2.1.0 -fastapi==0.113.0 +fastapi==0.114.2 fastapi-cli==0.0.5 fasteners==0.19 -filelock==3.15.4 +filelock==3.16.0 flexcache==0.3 flexparser==0.3.1 fonttools==4.53.1 @@ -77,12 +77,12 @@ httpcore==1.0.5 httptools==0.6.1 httpx==0.27.2 humanize==4.10.0 -identify==2.6.0 -idna==3.8 +identify==2.6.1 +idna==3.10 imageio==2.35.1 imagesize==1.4.1 -importlib_metadata==8.4.0 -importlib_resources==6.4.4 +importlib_metadata==8.5.0 +importlib_resources==6.4.5 iniconfig==2.0.0 intake==0.6.4 ipython==8.18.0 @@ -105,14 +105,14 @@ MarkupSafe==2.1.5 marshmallow==3.22.0 matplotlib==3.9.2 matplotlib-inline==0.1.7 -mdit-py-plugins==0.4.1 +mdit-py-plugins==0.4.2 mdurl==0.1.2 mistune==3.0.2 mock==5.1.0 mongoquery==1.4.2 -msgpack==1.0.8 +msgpack==1.1.0 msgpack-numpy==0.4.8 -multidict==6.0.5 +multidict==6.1.0 mypy==1.11.2 mypy-extensions==1.0.0 myst-parser==4.0.0 @@ -139,8 +139,8 @@ pika==1.3.2 pillow==10.4.0 PIMS==0.7 Pint==0.24.3 -pipdeptree==2.23.1 -platformdirs==4.2.2 +pipdeptree==2.23.3 +platformdirs==4.3.3 pluggy==1.5.0 plumbum==1.8.3 ply==3.11 @@ -152,12 +152,12 @@ ptyprocess==0.7.0 pure_eval==0.2.3 pvxslibs==1.3.1 py==1.11.0 -pyasn1==0.6.0 +pyasn1==0.6.1 pycryptodome==3.20.0 -pydantic==2.9.0 +pydantic==2.9.1 pydantic-extra-types==2.9.0 -pydantic-settings==2.4.0 -pydantic_core==2.23.2 +pydantic-settings==2.5.2 +pydantic_core==2.23.3 pydantic_numpy==5.0.2 pydata-sphinx-theme==0.15.4 pyepics==3.5.7 @@ -165,13 +165,13 @@ Pygments==2.18.0 pymongo==4.8.0 pyOlog==4.5.0 pyparsing==3.1.4 -pytest==8.3.2 +pytest==8.3.3 pytest-asyncio==0.24.0 pytest-cov==5.0.0 python-dateutil==2.9.0.post0 python-dotenv==1.0.1 python-multipart==0.0.9 -pytz==2024.1 +pytz==2024.2 PyYAML==6.0.2 pyyaml-include==2.1 questionary==2.0.1 @@ -184,7 +184,7 @@ rich==13.7.1 rpds-py==0.20.0 ruamel.yaml==0.18.6 ruamel.yaml.clib==0.2.8 -ruff==0.6.4 +ruff==0.6.5 scanspec==0.7.2 semver==3.0.2 setuptools-dso==2.11 @@ -210,7 +210,7 @@ sphinxcontrib-openapi==0.8.4 sphinxcontrib-qthelp==2.0.0 sphinxcontrib-serializinghtml==2.0.0 stack-data==0.6.3 -starlette==0.38.4 +starlette==0.38.5 stomp-py==8.1.2 suitcase-mongo==0.6.0 suitcase-msgpack==0.3.0 @@ -226,27 +226,27 @@ tqdm==4.66.5 traitlets==5.14.3 typer==0.12.4 types-mock==5.1.0.20240425 -types-PyYAML==6.0.12.20240808 -types-requests==2.32.0.20240905 +types-PyYAML==6.0.12.20240917 +types-requests==2.32.0.20240914 types-urllib3==1.26.25.14 typing-inspect==0.9.0 typing_extensions==4.12.2 tzdata==2024.1 tzlocal==5.2 ujson==5.10.0 -urllib3==2.2.2 +urllib3==2.2.3 uvicorn==0.30.6 uvloop==0.19.0 -virtualenv==20.26.3 +virtualenv==20.26.4 watchfiles==0.24.0 wcwidth==0.2.13 websocket-client==1.8.0 websockets==13.0.1 widgetsnbextension==4.0.13 workflows==2.27 -xarray==2024.7.0 -yarl==1.9.11 +xarray==2024.9.0 +yarl==1.11.1 zarr==2.18.3 zict==2.2.0 -zipp==3.20.1 +zipp==3.20.2 zocalo==1.1.0 diff --git a/helm/blueapi/values.yaml b/helm/blueapi/values.yaml index 4473907fd..d32e9f1eb 100644 --- a/helm/blueapi/values.yaml +++ b/helm/blueapi/values.yaml @@ -25,18 +25,16 @@ serviceAccount: podAnnotations: {} -podSecurityContext: - {} - # fsGroup: 2000 - -securityContext: - {} - # capabilities: - # drop: - # - ALL - # readOnlyRootFilesystem: true - # runAsNonRoot: true - # runAsUser: 1000 +podSecurityContext: {} +# fsGroup: 2000 + +securityContext: {} +# capabilities: +# drop: +# - ALL +# readOnlyRootFilesystem: true +# runAsNonRoot: true +# runAsUser: 1000 # Recommended for production to change service.type to ClusterIP and set up an Ingress service: @@ -47,23 +45,21 @@ ingress: create: false # host: foo.diamond.ac.uk (assumes port = service.port) -resources: - {} - # We usually recommend not to specify default resources and to leave this as a conscious - # choice for the user. This also increases chances charts run on environments with little - # resources, such as Minikube. If you do want to specify resources, uncomment the following - # lines, adjust them as necessary, and remove the curly braces after 'resources:'. - # limits: - # cpu: 100m - # memory: 128Mi - # requests: - # cpu: 100m - # memory: 128Mi - -initResources: - {} - # Can optionally specify separate resource constraints for the scratch setup container. - # If left empty this defaults to the same as resources above. +resources: {} +# We usually recommend not to specify default resources and to leave this as a conscious +# choice for the user. This also increases chances charts run on environments with little +# resources, such as Minikube. If you do want to specify resources, uncomment the following +# lines, adjust them as necessary, and remove the curly braces after 'resources:'. +# limits: +# cpu: 100m +# memory: 128Mi +# requests: +# cpu: 100m +# memory: 128Mi + +initResources: {} +# Can optionally specify separate resource constraints for the scratch setup container. +# If left empty this defaults to the same as resources above. nodeSelector: {} @@ -80,13 +76,12 @@ listener: resources: {} # Additional envVars to mount to the pod as a String -extraEnvVars: - [] - # - name: RABBITMQ_PASSWORD - # valueFrom: - # secretKeyRef: - # name: rabbitmq-password - # key: rabbitmq-password +extraEnvVars: [] +# - name: RABBITMQ_PASSWORD +# valueFrom: +# secretKeyRef: +# name: rabbitmq-password +# key: rabbitmq-password # Config for the worker goes here, will be mounted into a config file worker: @@ -95,18 +90,18 @@ worker: port: 8000 env: sources: - - kind: deviceFunctions - module: blueapi.startup.example_devices - - kind: planFunctions - module: blueapi.startup.example_plans - - kind: planFunctions - module: dls_bluesky_core.plans - - kind: planFunctions - module: dls_bluesky_core.stubs + - kind: deviceFunctions + module: blueapi.startup.example_devices + - kind: planFunctions + module: blueapi.startup.example_plans + - kind: planFunctions + module: dls_bluesky_core.plans + - kind: planFunctions + module: dls_bluesky_core.stubs stomp: auth: username: guest - passcode: guest + password: guest host: rabbitmq port: 61613 @@ -114,8 +109,8 @@ initContainer: scratch: root: /blueapi-plugins/scratch repositories: [] - # - name: "dodal" - # remote_url: https://github.com/DiamondLightSource/dodal.git + # - name: "dodal" + # remote_url: https://github.com/DiamondLightSource/dodal.git # Mount path for scratch area from host machine, setting # this effectively enables scratch area management diff --git a/pyproject.toml b/pyproject.toml index dc00ef316..ea16df3a0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,7 +31,7 @@ dependencies = [ "dls-dodal>=1.31.0", "super-state-machine", # See GH issue 553 "GitPython", - "bluesky-stomp>=0.1.0", + "bluesky-stomp>=0.1.2", ] dynamic = ["version"] license.file = "LICENSE" diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 9a86b9a92..c0069214d 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -7,7 +7,7 @@ import click from bluesky.callbacks.best_effort import BestEffortCallback -from bluesky_stomp.messaging import MessageContext, MessagingTemplate +from bluesky_stomp.messaging import MessageContext, StompClient from bluesky_stomp.models import Broker from pydantic import ValidationError from requests.exceptions import ConnectionError @@ -148,7 +148,7 @@ def listen_to_events(obj: dict) -> None: config: ApplicationConfig = obj["config"] if config.stomp is not None: event_bus_client = EventBusClient( - MessagingTemplate.for_broker( + StompClient.for_broker( broker=Broker( host=config.stomp.host, port=config.stomp.port, diff --git a/src/blueapi/client/client.py b/src/blueapi/client/client.py index 97dbf35a5..4468bf10e 100644 --- a/src/blueapi/client/client.py +++ b/src/blueapi/client/client.py @@ -1,7 +1,7 @@ import time from concurrent.futures import Future -from bluesky_stomp.messaging import MessageContext, MessagingTemplate +from bluesky_stomp.messaging import MessageContext, StompClient from bluesky_stomp.models import Broker from blueapi.config import ApplicationConfig @@ -41,7 +41,7 @@ def __init__( def from_config(cls, config: ApplicationConfig) -> "BlueapiClient": rest = BlueapiRestClient(config.api) if config.stomp is not None: - template = MessagingTemplate.for_broker( + template = StompClient.for_broker( broker=Broker( host=config.stomp.host, port=config.stomp.port, diff --git a/src/blueapi/client/event_bus.py b/src/blueapi/client/event_bus.py index 94e374d4b..b9516ee70 100644 --- a/src/blueapi/client/event_bus.py +++ b/src/blueapi/client/event_bus.py @@ -1,6 +1,6 @@ from collections.abc import Callable -from bluesky_stomp.messaging import MessageContext, MessagingTemplate +from bluesky_stomp.messaging import MessageContext, StompClient from bluesky_stomp.models import MessageTopic from blueapi.core import DataEvent @@ -17,9 +17,9 @@ def __init__(self, message: str) -> None: class EventBusClient: - app: MessagingTemplate + app: StompClient - def __init__(self, app: MessagingTemplate) -> None: + def __init__(self, app: StompClient) -> None: self.app = app def __enter__(self) -> None: diff --git a/src/blueapi/config.py b/src/blueapi/config.py index 98d0ef1e9..3502590ba 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -1,11 +1,11 @@ -import os from collections.abc import Mapping from enum import Enum from pathlib import Path from typing import Any, Generic, Literal, TypeVar import yaml -from pydantic import BaseModel, Field, TypeAdapter, ValidationError, field_validator +from bluesky_stomp.models import BasicAuthentication +from pydantic import BaseModel, Field, TypeAdapter, ValidationError from blueapi.utils import BlueapiBaseModel, InvalidConfigError @@ -23,25 +23,6 @@ class Source(BaseModel): module: Path | str -class BasicAuthentication(BaseModel): - """ - Log in details for when a server uses authentication. - If username or passcode match exactly the regex ^\\${(.*)}$ - they attempt to replace with an environment variable of the same. - i.e. ${foo} or ${FOO} are replaced with the value of FOO - """ - - username: str = "guest" - passcode: str = "guest" - - @field_validator("username", "passcode") - @classmethod - def get_from_env(cls, v: str): - if v.startswith("${") and v.endswith("}"): - return os.environ[v.removeprefix("${").removesuffix("}").upper()] - return v - - class StompConfig(BaseModel): """ Config for connecting to stomp broker diff --git a/src/blueapi/service/interface.py b/src/blueapi/service/interface.py index 72d546831..914639c62 100644 --- a/src/blueapi/service/interface.py +++ b/src/blueapi/service/interface.py @@ -3,7 +3,7 @@ from functools import lru_cache from typing import Any -from bluesky_stomp.messaging import MessagingTemplate +from bluesky_stomp.messaging import StompClient from bluesky_stomp.models import Broker, DestinationBase, MessageTopic from blueapi.config import ApplicationConfig @@ -49,10 +49,10 @@ def worker() -> TaskWorker: @lru_cache -def messaging_template() -> MessagingTemplate | None: +def messaging_template() -> StompClient | None: stomp_config = config().stomp if stomp_config is not None: - template = MessagingTemplate.for_broker( + template = StompClient.for_broker( broker=Broker( host=stomp_config.host, port=stomp_config.port, auth=stomp_config.auth ) diff --git a/tests/unit_tests/client/test_event_bus.py b/tests/unit_tests/client/test_event_bus.py index ff5eb3101..34ac41a63 100644 --- a/tests/unit_tests/client/test_event_bus.py +++ b/tests/unit_tests/client/test_event_bus.py @@ -1,18 +1,18 @@ from unittest.mock import ANY, Mock import pytest -from bluesky_stomp.messaging import MessagingTemplate +from bluesky_stomp.messaging import StompClient from blueapi.client.event_bus import BlueskyStreamingError, EventBusClient @pytest.fixture -def mock_template() -> MessagingTemplate: - return Mock(spec=MessagingTemplate) +def mock_template() -> StompClient: + return Mock(spec=StompClient) @pytest.fixture -def events(mock_template: MessagingTemplate) -> EventBusClient: +def events(mock_template: StompClient) -> EventBusClient: return EventBusClient(app=mock_template) diff --git a/tests/unit_tests/service/test_interface.py b/tests/unit_tests/service/test_interface.py index 002d2d308..86dd2e5c1 100644 --- a/tests/unit_tests/service/test_interface.py +++ b/tests/unit_tests/service/test_interface.py @@ -3,7 +3,7 @@ from unittest.mock import MagicMock, Mock, patch import pytest -from bluesky_stomp.messaging import MessagingTemplate +from bluesky_stomp.messaging import StompClient from ophyd.sim import SynAxis from stomp.connect import StompConnection11 as Connection @@ -23,8 +23,8 @@ def mock_connection() -> Mock: @pytest.fixture -def template(mock_connection: Mock) -> MessagingTemplate: - template = MessagingTemplate(conn=mock_connection) +def template(mock_connection: Mock) -> StompClient: + template = StompClient(conn=mock_connection) template.disconnect = MagicMock() return template @@ -283,9 +283,9 @@ def test_get_task_by_id(context_mock: MagicMock): ) -def test_stomp_config(template: MessagingTemplate): +def test_stomp_config(template: StompClient): with patch( - "blueapi.service.interface.MessagingTemplate.for_broker", return_value=template + "blueapi.service.interface.StompClient.for_broker", return_value=template ): interface.set_config(ApplicationConfig(stomp=StompConfig())) assert interface.messaging_template() is not None diff --git a/tests/unit_tests/test_cli.py b/tests/unit_tests/test_cli.py index afc0cdece..370a90e85 100644 --- a/tests/unit_tests/test_cli.py +++ b/tests/unit_tests/test_cli.py @@ -9,7 +9,7 @@ import pytest import responses -from bluesky_stomp.messaging import MessagingTemplate +from bluesky_stomp.messaging import StompClient from click.testing import CliRunner from pydantic import BaseModel, ValidationError from requests.exceptions import ConnectionError @@ -38,8 +38,8 @@ def mock_connection() -> Mock: @pytest.fixture -def template(mock_connection: Mock) -> MessagingTemplate: - return MessagingTemplate(conn=mock_connection) +def template(mock_connection: Mock) -> StompClient: + return StompClient(conn=mock_connection) @pytest.fixture @@ -150,9 +150,9 @@ def test_cannot_run_plans_without_stomp_config(runner: CliRunner): ) -@patch("blueapi.cli.cli.MessagingTemplate") +@patch("blueapi.cli.cli.StompClient") def test_valid_stomp_config_for_listener( - template: MessagingTemplate, + template: StompClient, runner: CliRunner, mock_connection: Mock, ): diff --git a/tests/unit_tests/test_config.py b/tests/unit_tests/test_config.py index cb446ea40..d4ffa10c1 100644 --- a/tests/unit_tests/test_config.py +++ b/tests/unit_tests/test_config.py @@ -4,9 +4,10 @@ from unittest import mock import pytest +from bluesky_stomp.models import BasicAuthentication from pydantic import BaseModel, Field -from blueapi.config import BasicAuthentication, ConfigLoader +from blueapi.config import ConfigLoader from blueapi.utils import InvalidConfigError @@ -121,28 +122,28 @@ def test_error_thrown_if_schema_does_not_match_yaml(nested_config_yaml: Path) -> @mock.patch.dict(os.environ, {"FOO": "bar"}, clear=True) def test_auth_from_env(): - auth = BasicAuthentication(username="${FOO}", passcode="baz") + auth = BasicAuthentication(username="${FOO}", password="baz") assert auth.username == "bar" @mock.patch.dict(os.environ, {"FOO": "bar", "BAZ": "qux"}, clear=True) def test_auth_from_env_repeated_key(): - auth = BasicAuthentication(username="${FOO}", passcode="${FOO}") + auth = BasicAuthentication(username="${FOO}", password="${FOO}") assert auth.username == "bar" - assert auth.passcode == "bar" + assert auth.password.get_secret_value() == "bar" @mock.patch.dict(os.environ, {"FOO": "bar"}, clear=True) def test_auth_from_env_ignore_case(): - auth = BasicAuthentication(username="${FOO}", passcode="${foo}") + auth = BasicAuthentication(username="${FOO}", password="${foo}") assert auth.username == "bar" - assert auth.passcode == "bar" + assert auth.password.get_secret_value() == "bar" @mock.patch.dict(os.environ, {"FOO": "bar"}, clear=True) def test_auth_from_env_throws_when_not_available(): # Eagerly throws an exception, will fail during initial loading with pytest.raises(KeyError): - BasicAuthentication(username="${BAZ}", passcode="baz") + BasicAuthentication(username="${BAZ}", password="baz") with pytest.raises(KeyError): - BasicAuthentication(username="${baz}", passcode="baz") + BasicAuthentication(username="${baz}", password="baz")