diff --git a/docs/reference/openapi.yaml b/docs/reference/openapi.yaml index 57a4d9352..8ffcf2217 100644 --- a/docs/reference/openapi.yaml +++ b/docs/reference/openapi.yaml @@ -37,6 +37,12 @@ components: additionalProperties: false description: State of internal environment. properties: + environment_id: + description: Unique ID for the environment instance, can be used to differentiate + between a new environment and old that has been torn down + format: uuid + title: Environment Id + type: string error_message: anyOf: - minLength: 1 @@ -49,6 +55,7 @@ components: title: Initialized type: boolean required: + - environment_id - initialized title: EnvironmentResponse type: object diff --git a/src/blueapi/client/client.py b/src/blueapi/client/client.py index 5bdbc06f1..f7c44fda5 100644 --- a/src/blueapi/client/client.py +++ b/src/blueapi/client/client.py @@ -386,7 +386,9 @@ def reload_environment( """ try: + # _ = self._rest.get_environment() status = self._rest.delete_environment() + except Exception as e: raise BlueskyRemoteControlError( "Failed to tear down the environment" diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 37a41f30d..9daf2a169 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -166,10 +166,11 @@ async def delete_environment( runner: WorkerDispatcher = Depends(_runner), ) -> EnvironmentResponse: """Delete the current environment, causing internal components to be reloaded.""" - if runner.state.initialized or runner.state.error_message is not None: background_tasks.add_task(runner.reload) - return EnvironmentResponse(initialized=False) + return EnvironmentResponse( + environment_id=runner.state.environment_id, initialized=False + ) @auth_router.get("/config/oidc", tags=["auth"], response_model=OIDCConfig) diff --git a/src/blueapi/service/model.py b/src/blueapi/service/model.py index 23dd667a0..8f0e9d60b 100644 --- a/src/blueapi/service/model.py +++ b/src/blueapi/service/model.py @@ -1,6 +1,6 @@ +import uuid from collections.abc import Iterable from typing import Any -from uuid import UUID from bluesky.protocols import HasName from pydantic import Field @@ -145,7 +145,7 @@ class EnvironmentResponse(BlueapiBaseModel): State of internal environment. """ - environment_id: UUID = Field( + environment_id: uuid.UUID = Field( description="Unique ID for the environment instance, can be used to " "differentiate between a new environment and old that has been torn down" ) diff --git a/tests/system_tests/test_blueapi_system.py b/tests/system_tests/test_blueapi_system.py index 61656d253..39beef408 100644 --- a/tests/system_tests/test_blueapi_system.py +++ b/tests/system_tests/test_blueapi_system.py @@ -19,7 +19,6 @@ ) from blueapi.service.model import ( DeviceResponse, - EnvironmentResponse, PlanResponse, TaskResponse, WorkerTask, @@ -335,9 +334,9 @@ def on_event(event: AnyEvent): def test_get_current_state_of_environment(client: BlueapiClient): - assert client.get_environment() == EnvironmentResponse(initialized=True) + assert client.get_environment().initialized def test_delete_current_environment(client: BlueapiClient): client.reload_environment() - assert client.get_environment() == EnvironmentResponse(initialized=True) + assert client.get_environment().initialized diff --git a/tests/unit_tests/client/test_client.py b/tests/unit_tests/client/test_client.py index 287aad45a..c3683579d 100644 --- a/tests/unit_tests/client/test_client.py +++ b/tests/unit_tests/client/test_client.py @@ -1,3 +1,4 @@ +import uuid from collections.abc import Callable from unittest.mock import MagicMock, Mock, call, patch @@ -42,7 +43,8 @@ TASK = TrackableTask(task_id="foo", task=Task(name="bar", params={})) TASKS = TasksListResponse(tasks=[TASK]) ACTIVE_TASK = WorkerTask(task_id="bar") -ENV = EnvironmentResponse(initialized=True) +ENVIRONMENT_ID = uuid.uuid4() +ENV = EnvironmentResponse(environment_id=ENVIRONMENT_ID, initialized=True) COMPLETE_EVENT = WorkerEvent( state=WorkerState.IDLE, task_status=TaskStatus( @@ -74,7 +76,9 @@ def mock_rest() -> BlueapiRestClient: mock.get_all_tasks.return_value = TASKS mock.get_active_task.return_value = ACTIVE_TASK mock.get_environment.return_value = ENV - mock.delete_environment.return_value = EnvironmentResponse(initialized=False) + mock.delete_environment.return_value = EnvironmentResponse( + environment_id=ENVIRONMENT_ID, initialized=False + ) return mock @@ -268,10 +272,10 @@ def test_reload_environment_no_timeout( mock_rest: Mock, ): mock_rest.get_environment.side_effect = [ - EnvironmentResponse(initialized=False), - EnvironmentResponse(initialized=False), - EnvironmentResponse(initialized=False), - EnvironmentResponse(initialized=True), + EnvironmentResponse(environment_id=ENVIRONMENT_ID, initialized=False), + EnvironmentResponse(environment_id=ENVIRONMENT_ID, initialized=False), + EnvironmentResponse(environment_id=ENVIRONMENT_ID, initialized=False), + EnvironmentResponse(environment_id=ENVIRONMENT_ID, initialized=True), ] mock_time.return_value = 100.0 client.reload_environment(timeout=None) @@ -287,10 +291,10 @@ def test_reload_environment_with_timeout( mock_rest: Mock, ): mock_rest.get_environment.side_effect = [ - EnvironmentResponse(initialized=False), - EnvironmentResponse(initialized=False), - EnvironmentResponse(initialized=False), - EnvironmentResponse(initialized=False), + EnvironmentResponse(environment_id=ENVIRONMENT_ID, initialized=False), + EnvironmentResponse(environment_id=ENVIRONMENT_ID, initialized=False), + EnvironmentResponse(environment_id=ENVIRONMENT_ID, initialized=False), + EnvironmentResponse(environment_id=ENVIRONMENT_ID, initialized=False), ] mock_time.side_effect = [ 100.0, @@ -315,10 +319,14 @@ def test_reload_environment_ignores_current_environment( mock_rest: Mock, ): mock_rest.get_environment.side_effect = [ - EnvironmentResponse(initialized=True), # This is the old environment - EnvironmentResponse(initialized=False), - EnvironmentResponse(initialized=False), - EnvironmentResponse(initialized=True), # This is the new environment + EnvironmentResponse( + environment_id=ENVIRONMENT_ID, initialized=True + ), # This is the old environment + EnvironmentResponse(environment_id=ENVIRONMENT_ID, initialized=False), + EnvironmentResponse(environment_id=ENVIRONMENT_ID, initialized=False), + EnvironmentResponse( + environment_id=ENVIRONMENT_ID, initialized=True + ), # This is the new environment ] mock_time.return_value = 100.0 client.reload_environment(timeout=None) @@ -330,7 +338,7 @@ def test_reload_environment_failure( mock_rest: Mock, ): mock_rest.get_environment.return_value = EnvironmentResponse( - initialized=False, error_message="foo" + environment_id=ENVIRONMENT_ID, initialized=False, error_message="foo" ) with pytest.raises(BlueskyRemoteControlError, match="foo"): client.reload_environment() diff --git a/tests/unit_tests/client/test_rest.py b/tests/unit_tests/client/test_rest.py index 2eddafe22..09510a6ba 100644 --- a/tests/unit_tests/client/test_rest.py +++ b/tests/unit_tests/client/test_rest.py @@ -1,3 +1,4 @@ +import uuid from pathlib import Path from unittest.mock import Mock, patch @@ -53,16 +54,21 @@ def test_auth_request_functionality( mock_authn_server: responses.RequestsMock, cached_valid_token: Path, ): + environment_id = uuid.uuid4() mock_authn_server.stop() # Cannot use multiple RequestsMock context manager mock_get_env = mock_authn_server.get( "http://localhost:8000/environment", - json=EnvironmentResponse(initialized=True).model_dump(), + json=EnvironmentResponse( + environment_id=environment_id, initialized=True + ).model_dump(mode="json"), status=200, ) result = None with mock_authn_server: result = rest_with_auth.get_environment() - assert result == EnvironmentResponse(initialized=True) + assert result == EnvironmentResponse( + environment_id=environment_id, initialized=True + ) calls = mock_get_env.calls assert len(calls) == 1 cacheManager = SessionCacheManager(cached_valid_token) @@ -75,16 +81,21 @@ def test_refresh_if_signature_expired( mock_authn_server: responses.RequestsMock, cached_valid_refresh: Path, ): + environment_id = uuid.uuid4() mock_authn_server.stop() # Cannot use multiple RequestsMock context manager mock_get_env = mock_authn_server.get( "http://localhost:8000/environment", - json=EnvironmentResponse(initialized=True).model_dump(), + json=EnvironmentResponse( + environment_id=environment_id, initialized=True + ).model_dump(mode="json"), status=200, ) result = None with mock_authn_server: result = rest_with_auth.get_environment() - assert result == EnvironmentResponse(initialized=True) + assert result == EnvironmentResponse( + environment_id=environment_id, initialized=True + ) calls = mock_get_env.calls assert len(calls) == 1 assert calls[0].request.headers["Authorization"] == "Bearer new_token" diff --git a/tests/unit_tests/service/test_rest_api.py b/tests/unit_tests/service/test_rest_api.py index c18e98a68..58f616586 100644 --- a/tests/unit_tests/service/test_rest_api.py +++ b/tests/unit_tests/service/test_rest_api.py @@ -535,12 +535,15 @@ def test_set_state_invalid_transition(mock_runner: Mock, client: TestClient): def test_get_environment_idle(mock_runner: Mock, client: TestClient) -> None: + environment_id = uuid.uuid4() mock_runner.state = EnvironmentResponse( + environment_id=environment_id, initialized=True, error_message=None, ) assert client.get("/environment").json() == { + "environment_id": str(environment_id), "initialized": True, "error_message": None, } diff --git a/tests/unit_tests/service/test_runner.py b/tests/unit_tests/service/test_runner.py index 2c7c4b7b0..0cdfe477a 100644 --- a/tests/unit_tests/service/test_runner.py +++ b/tests/unit_tests/service/test_runner.py @@ -1,3 +1,4 @@ +import uuid from multiprocessing.pool import Pool as PoolClass from typing import Any, Generic, TypeVar from unittest.mock import MagicMock, Mock, patch @@ -72,7 +73,9 @@ def test_raises_if_used_before_started(runner: WorkerDispatcher): def test_error_on_runner_setup(runner: WorkerDispatcher, mock_subprocess: Mock): error_message = "Intentional start_worker exception" + environment_id = uuid.uuid4() expected_state = EnvironmentResponse( + environment_id=environment_id, initialized=False, error_message=error_message, ) @@ -83,6 +86,7 @@ def test_error_on_runner_setup(runner: WorkerDispatcher, mock_subprocess: Mock): # and the runner is not yet initialised runner.reload() state = runner.state + expected_state.environment_id = state.environment_id assert state == expected_state @@ -110,14 +114,17 @@ def test_can_reload_after_an_error(pool_mock: MagicMock): runner = WorkerDispatcher() runner.start() - + current_env = runner.state.environment_id assert runner.state == EnvironmentResponse( - initialized=False, error_message="invalid code" + environment_id=current_env, initialized=False, error_message="invalid code" ) runner.reload() - - assert runner.state == EnvironmentResponse(initialized=True, error_message=None) + new_env = runner.state.environment_id + assert runner.state == EnvironmentResponse( + environment_id=new_env, initialized=True, error_message=None + ) + assert current_env != new_env @patch("blueapi.service.runner.Pool") diff --git a/tests/unit_tests/test_cli.py b/tests/unit_tests/test_cli.py index 91686756e..7240a5f4d 100644 --- a/tests/unit_tests/test_cli.py +++ b/tests/unit_tests/test_cli.py @@ -1,4 +1,5 @@ import json +import uuid from collections.abc import Mapping from dataclasses import dataclass from io import StringIO @@ -219,15 +220,21 @@ def test_valid_stomp_config_for_listener( def test_get_env( runner: CliRunner, ): + environment_id = uuid.uuid4() responses.add( responses.GET, "http://localhost:8000/environment", - json=EnvironmentResponse(initialized=True).model_dump(), + json=EnvironmentResponse( + environment_id=environment_id, initialized=True + ).model_dump(mode="json"), status=200, ) env = runner.invoke(main, ["controller", "env"]) - assert env.output == "initialized=True error_message=None\n" + assert ( + env.output + == f"environment_id=UUID('{environment_id}') initialized=True error_message=None\n" # noqa: E501 + ) @responses.activate(assert_all_requests_are_fired=True) @@ -236,20 +243,25 @@ def test_reset_env_client_behavior( mock_sleep: Mock, runner: CliRunner, ): + environment_id = uuid.uuid4() responses.add( responses.DELETE, "http://localhost:8000/environment", - json=EnvironmentResponse(initialized=False).model_dump(), + json=EnvironmentResponse( + environment_id=environment_id, initialized=False + ).model_dump(mode="json"), status=200, ) env_state = [False, False, True] - + environment_id = uuid.uuid4() for state in env_state: responses.add( responses.GET, "http://localhost:8000/environment", - json=EnvironmentResponse(initialized=state).model_dump(), + json=EnvironmentResponse( + environment_id=environment_id, initialized=state + ).model_dump(mode="json"), status=200, ) @@ -269,28 +281,33 @@ def test_reset_env_client_behavior( # Check if the final environment status is printed correctly # assert "Environment is initialized." in result.output - assert reload_result.output == dedent("""\ + assert reload_result.output == dedent(f"""\ Reloading environment Environment is initialized - initialized=True error_message=None - """) + environment_id=UUID('{environment_id}') initialized=True error_message=None + """) # noqa: E501 @responses.activate @patch("blueapi.client.client.time.sleep", return_value=None) def test_env_timeout(mock_sleep: Mock, runner: CliRunner): # Setup mocked responses for the REST endpoints + environment_id = uuid.uuid4() responses.add( responses.DELETE, "http://localhost:8000/environment", status=200, - json=EnvironmentResponse(initialized=False).model_dump(), + json=EnvironmentResponse( + environment_id=environment_id, initialized=False + ).model_dump(mode="json"), ) # Add responses for each polling attempt, all indicating not initialized responses.add( responses.GET, "http://localhost:8000/environment", - json=EnvironmentResponse(initialized=False).model_dump(), + json=EnvironmentResponse( + environment_id=environment_id, initialized=False + ).model_dump(mode="json"), status=200, )