From 93119d95f6c6efbe85bcf39ffb6172ef8a1724f0 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Thu, 14 Mar 2024 09:44:00 +0000 Subject: [PATCH 01/38] ran into __root__ and Pydantic None error --- src/blueapi/cli/cli.py | 22 ++++++++++++++++++++++ src/blueapi/cli/rest.py | 14 +++++++++++++- src/blueapi/service/main.py | 5 +++-- 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 5a293902a..d77116ca4 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -278,6 +278,28 @@ def stop(obj: dict) -> None: pprint(client.cancel_current_task(state=WorkerState.STOPPING)) +@controller.command(name="env") +@check_connection +@click.option( + "-r", + "--reload", + is_flag=True, + type=bool, + help="Reload the current environment", + default=False, +) +@click.pass_obj +def env(obj: dict, reload: Optional[bool]) -> None: + """ + Inspect or restart the environment + """ + + client: BlueapiRestClient = obj["rest_client"] + if reload: + pprint(client.reload_environemnt()) + pprint(client.get_environment()) + + # helper function def process_event_after_finished(event: WorkerEvent, logger: logging.Logger): if event.is_error(): diff --git a/src/blueapi/cli/rest.py b/src/blueapi/cli/rest.py index 420ebd13e..46b88fbd3 100644 --- a/src/blueapi/cli/rest.py +++ b/src/blueapi/cli/rest.py @@ -9,6 +9,7 @@ from blueapi.service.model import ( DeviceModel, DeviceResponse, + EnvironmentResponse, PlanModel, PlanResponse, TaskResponse, @@ -115,7 +116,10 @@ def _request_and_deserialize( raise_if: Callable[[requests.Response], bool] = _is_exception, ) -> T: url = self._url(suffix) - response = requests.request(method, url, json=data) + if data: + response = requests.request(method, url, json=data) + else: + response = requests.request(method, url) if raise_if(response): message = get_status_message(response.status_code) error_message = f"""Response failed with text: {response.text}, @@ -128,3 +132,11 @@ def _request_and_deserialize( def _url(self, suffix: str) -> str: base_url = f"{self._config.protocol}://{self._config.host}:{self._config.port}" return f"{base_url}{suffix}" + + def get_environment(self) -> EnvironmentResponse: + return self._request_and_deserialize("/environment", EnvironmentResponse) + + def reload_environemnt(self) -> EnvironmentResponse: + return self._request_and_deserialize( + "/environment", EnvironmentResponse, method="DELETE" + ) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 1eeb0752a..b5e8edca3 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -89,17 +89,18 @@ def get_environment( return EnvironmentResponse(initialized=handler.initialized) -@app.delete("/environment") +@app.delete("/environment", response_model=EnvironmentResponse) async def delete_environment( background_tasks: BackgroundTasks, handler: BlueskyHandler = Depends(get_handler), -): +) -> EnvironmentResponse: def restart_handler(handler: BlueskyHandler): handler.stop() handler.start() if handler.initialized: background_tasks.add_task(restart_handler, handler) + return EnvironmentResponse(initialized=False) @app.get("/plans", response_model=PlanResponse) From 1c22fc83fdcdd7b0e8a8424eec616338d4162b96 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Thu, 28 Mar 2024 12:05:46 +0000 Subject: [PATCH 02/38] change the vscode settings to explicit --- src/blueapi/cli/cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index d77116ca4..78dac556b 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -296,6 +296,7 @@ def env(obj: dict, reload: Optional[bool]) -> None: client: BlueapiRestClient = obj["rest_client"] if reload: + print('reloading the environment...') pprint(client.reload_environemnt()) pprint(client.get_environment()) From c6642b737c400abde755f355f6e6a562fef95165 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Tue, 2 Apr 2024 09:43:57 +0100 Subject: [PATCH 03/38] add tests --- .vscode/settings.json | 26 ++++++++++++-------- tests/test_cli.py | 55 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 10 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index c129d991b..918d8347c 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,11 +1,17 @@ { - "python.testing.unittestEnabled": false, - "python.testing.pytestEnabled": true, - "editor.formatOnSave": true, - "editor.codeActionsOnSave": { - "source.organizeImports": "explicit" - }, - "[python]": { - "editor.defaultFormatter": "charliermarsh.ruff", - }, -} \ No newline at end of file + "python.linting.pylintEnabled": false, + "python.linting.flake8Enabled": true, + "python.linting.mypyEnabled": true, + "python.linting.enabled": true, + "python.testing.pytestArgs": ["tests"], + "python.testing.unittestEnabled": false, + "python.testing.pytestEnabled": true, + "python.formatting.provider": "black", + "python.languageServer": "Pylance", + "editor.formatOnSave": true, + "editor.codeActionsOnSave": { + "source.organizeImports": "explicit" + }, + "esbonio.server.enabled": true, + "esbonio.sphinx.confDir": "" +} diff --git a/tests/test_cli.py b/tests/test_cli.py index 762be122a..d98e1eeff 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -200,3 +200,58 @@ def test_valid_stomp_config_for_listener(runner: CliRunner): input="\n", ) assert result.exit_code == 0 + + +def test_invalid_condition_for_run(runner: CliRunner): + result = runner.invoke(main, ["controller", "run", "sleep", '{"time": 5}']) + assert type(result.exception) is RuntimeError + + +@pytest.mark.handler +@patch("blueapi.service.handler.Handler") +@patch("requests.request") +def test_get_env( + mock_requests: Mock, + mock_handler: Mock, + handler: Handler, + client: TestClient, + runner: CliRunner, +): + with patch("uvicorn.run", side_effect=None): + result = runner.invoke(main, ["serve"]) + + assert result.exit_code == 0 + + mock_requests.return_value = Mock() + + runner.invoke(main, ["controller", "env"]) + + assert mock_requests.call_args[0] == ( + "GET", + "http://localhost:8000/environment", + ) + + +@pytest.mark.handler +@patch("blueapi.service.handler.Handler") +@patch("requests.request") +def test_reset_env( + mock_requests: Mock, + mock_handler: Mock, + handler: Handler, + client: TestClient, + runner: CliRunner, +): + with patch("uvicorn.run", side_effect=None): + result = runner.invoke(main, ["serve"]) + + assert result.exit_code == 0 + + mock_requests.return_value = Mock() + + runner.invoke(main, ["controller", "env", "-r"]) + + assert mock_requests.call_args[0] == ( + "DELETE", + "http://localhost:8000/environment", + ) From 686221ff1d8d2607da294f7b34d4ad2665d6a4fb Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Tue, 2 Apr 2024 10:09:49 +0100 Subject: [PATCH 04/38] update tests and schema --- docs/reference/openapi.yaml | 3 ++- src/blueapi/cli/cli.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/reference/openapi.yaml b/docs/reference/openapi.yaml index 6922e0bc9..8fc1a75cd 100644 --- a/docs/reference/openapi.yaml +++ b/docs/reference/openapi.yaml @@ -256,7 +256,8 @@ paths: '200': content: application/json: - schema: {} + schema: + $ref: '#/components/schemas/EnvironmentResponse' description: Successful Response summary: Delete Environment get: diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 78dac556b..0b76d43e1 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -291,12 +291,12 @@ def stop(obj: dict) -> None: @click.pass_obj def env(obj: dict, reload: Optional[bool]) -> None: """ - Inspect or restart the environment + Inspect or restart the environment """ client: BlueapiRestClient = obj["rest_client"] if reload: - print('reloading the environment...') + print("reloading the environment...") pprint(client.reload_environemnt()) pprint(client.get_environment()) From eec09d78b5d896aad7580c428733faec0c629405 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Wed, 3 Apr 2024 12:54:21 +0000 Subject: [PATCH 05/38] add polling to environment reset --- src/blueapi/cli/cli.py | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 0b76d43e1..8566f5af0 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -4,6 +4,8 @@ from functools import wraps from pathlib import Path from pprint import pprint +from time import sleep +from typing import Optional import click from pydantic import ValidationError @@ -295,10 +297,32 @@ def env(obj: dict, reload: Optional[bool]) -> None: """ client: BlueapiRestClient = obj["rest_client"] - if reload: - print("reloading the environment...") - pprint(client.reload_environemnt()) - pprint(client.get_environment()) + if not reload: + pprint(client.get_environment()) + return + + # Reload the environment if needed + print("Reloading the environment...") + pprint(client.reload_environment()) + + # Initialize a variable to keep track of the environment status + environment_initialized = False + + # Use a while loop to keep checking until the environment is initialized + while not environment_initialized: + # Fetch the current environment status + environment_status = client.get_environment() + + # Check if the environment is initialized + if environment_status.initialized: + print("Environment is initialized.") + environment_initialized = True + else: + print("Waiting for environment to initialize...") + sleep(5) # Wait for 5 seconds before checking again + + # Once out of the loop, print the initialized environment status + pprint(environment_status) # helper function From a992f32055f582c25f94dbe91fbe22ed6c5be900 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Wed, 3 Apr 2024 13:52:53 +0000 Subject: [PATCH 06/38] error with patching sleepy test --- tests/test_cli.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/test_cli.py b/tests/test_cli.py index d98e1eeff..fe7a6f3d0 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -255,3 +255,33 @@ def test_reset_env( "DELETE", "http://localhost:8000/environment", ) + + +@pytest.mark.handler +@patch("blueapi.service.handler.Handler") +@patch("requests.request") +def test_reset_env2( + mock_requests: Mock, + mock_handler: Mock, + handler: Handler, + client: TestClient, + runner: CliRunner, +): + # Mock the sleep function to avoid actual delays in the test + with patch("time.sleep", return_value=None): + with patch("uvicorn.run", side_effect=None): + result = runner.invoke(main, ["serve"]) + + assert result.exit_code == 0 + + mock_requests.return_value = Mock() + + # Invoke the CLI command that would trigger the environment initialization check + runner.invoke(main, ["controller", "env", "-r"]) + + # Check if the DELETE request was made correctly + assert mock_requests.call_args[0] == ( + "DELETE", + "http://localhost:8000/environment", + ) + From 09e345cc99bd506cd46e43569644c1a87fc9edd1 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Wed, 3 Apr 2024 15:22:44 +0100 Subject: [PATCH 07/38] change timeout in the cli --- src/blueapi/cli/cli.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 8566f5af0..45e4eb62a 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -307,9 +307,10 @@ def env(obj: dict, reload: Optional[bool]) -> None: # Initialize a variable to keep track of the environment status environment_initialized = False - + polling_count = 0 + max_polling_count = 10 # Use a while loop to keep checking until the environment is initialized - while not environment_initialized: + while (not environment_initialized and polling_count < max_polling_count): # Fetch the current environment status environment_status = client.get_environment() @@ -319,7 +320,10 @@ def env(obj: dict, reload: Optional[bool]) -> None: environment_initialized = True else: print("Waiting for environment to initialize...") - sleep(5) # Wait for 5 seconds before checking again + polling_count += 1 + sleep(1) # Wait for 1 seconds before checking again + if(polling_count == max_polling_count): + raise TimeoutError("Environment initialization timed out.") # Once out of the loop, print the initialized environment status pprint(environment_status) From 11838c52c448297f7286753089380d3e07af2616 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Thu, 4 Apr 2024 13:29:09 +0000 Subject: [PATCH 08/38] lint --- .vscode/settings.json | 6 ++++-- src/blueapi/cli/cli.py | 4 ++-- src/blueapi/service/main.py | 4 +++- tests/test_cli.py | 26 ++++++++++++-------------- tests/worker/test_reworker.py | 4 +++- 5 files changed, 24 insertions(+), 20 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 918d8347c..81aac8ea9 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -3,7 +3,9 @@ "python.linting.flake8Enabled": true, "python.linting.mypyEnabled": true, "python.linting.enabled": true, - "python.testing.pytestArgs": ["tests"], + "python.testing.pytestArgs": [ + "tests" + ], "python.testing.unittestEnabled": false, "python.testing.pytestEnabled": true, "python.formatting.provider": "black", @@ -14,4 +16,4 @@ }, "esbonio.server.enabled": true, "esbonio.sphinx.confDir": "" -} +} \ No newline at end of file diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 45e4eb62a..cb5c2d938 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -310,7 +310,7 @@ def env(obj: dict, reload: Optional[bool]) -> None: polling_count = 0 max_polling_count = 10 # Use a while loop to keep checking until the environment is initialized - while (not environment_initialized and polling_count < max_polling_count): + while not environment_initialized and polling_count < max_polling_count: # Fetch the current environment status environment_status = client.get_environment() @@ -322,7 +322,7 @@ def env(obj: dict, reload: Optional[bool]) -> None: print("Waiting for environment to initialize...") polling_count += 1 sleep(1) # Wait for 1 seconds before checking again - if(polling_count == max_polling_count): + if polling_count == max_polling_count: raise TimeoutError("Environment initialization timed out.") # Once out of the loop, print the initialized environment status diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index b5e8edca3..cd4112912 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -141,7 +141,9 @@ def get_device_by_name(name: str, handler: BlueskyHandler = Depends(get_handler) def submit_task( request: Request, response: Response, - task: Task = Body(..., example=Task(name="count", params={"detectors": ["x"]})), # noqa: B008 + task: Task = Body( + ..., example=Task(name="count", params={"detectors": ["x"]}) + ), # noqa: B008 handler: BlueskyHandler = Depends(get_handler), ): """Submit a task to the worker.""" diff --git a/tests/test_cli.py b/tests/test_cli.py index fe7a6f3d0..286c202ca 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -260,6 +260,7 @@ def test_reset_env( @pytest.mark.handler @patch("blueapi.service.handler.Handler") @patch("requests.request") +@patch("time.sleep", return_value=None) def test_reset_env2( mock_requests: Mock, mock_handler: Mock, @@ -267,21 +268,18 @@ def test_reset_env2( client: TestClient, runner: CliRunner, ): - # Mock the sleep function to avoid actual delays in the test - with patch("time.sleep", return_value=None): - with patch("uvicorn.run", side_effect=None): - result = runner.invoke(main, ["serve"]) - - assert result.exit_code == 0 + with patch("uvicorn.run", side_effect=None): + result = runner.invoke(main, ["serve"]) - mock_requests.return_value = Mock() + assert result.exit_code == 0 - # Invoke the CLI command that would trigger the environment initialization check - runner.invoke(main, ["controller", "env", "-r"]) + mock_requests.return_value = Mock() - # Check if the DELETE request was made correctly - assert mock_requests.call_args[0] == ( - "DELETE", - "http://localhost:8000/environment", - ) + # Invoke the CLI command that would trigger the environment initialization check + runner.invoke(main, ["controller", "env", "-r"]) + # Check if the DELETE request was made correctly + assert mock_requests.call_args[0] == ( + "DELETE", + "http://localhost:8000/environment", + ) diff --git a/tests/worker/test_reworker.py b/tests/worker/test_reworker.py index 0f2b3be7b..0c8ab2495 100644 --- a/tests/worker/test_reworker.py +++ b/tests/worker/test_reworker.py @@ -346,7 +346,9 @@ def test_worker_and_data_events_produce_in_order(worker: Worker) -> None: def assert_running_count_plan_produces_ordered_worker_and_data_events( expected_events: list[WorkerEvent | DataEvent], worker: Worker, - task: Task = Task(name="count", params={"detectors": ["image_det"], "num": 1}), # noqa: B008 + task: Task = Task( + name="count", params={"detectors": ["image_det"], "num": 1} + ), # noqa: B008 timeout: float = 5.0, ) -> None: event_streams: list[EventStream[Any, int]] = [ From 32add10fb34dcf6d647de53dd4994ebf914dd116 Mon Sep 17 00:00:00 2001 From: David Perl Date: Wed, 24 Apr 2024 14:55:17 +0000 Subject: [PATCH 09/38] (#366) fix function reference and mocking in cli test --- .vscode/launch.json | 3 ++- src/blueapi/cli/cli.py | 2 +- src/blueapi/cli/rest.py | 2 +- tests/test_cli.py | 5 +++-- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index a82bb594c..24d07cab7 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -29,6 +29,7 @@ "env": { // Enable break on exception when debugging tests (see: tests/conftest.py) "PYTEST_RAISE": "1", + "PYTEST_ADDOPTS": "--no-cov -vv" }, }, { @@ -78,4 +79,4 @@ ] } ] -} +} \ No newline at end of file diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index cb5c2d938..e81c416ca 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -296,7 +296,7 @@ def env(obj: dict, reload: Optional[bool]) -> None: Inspect or restart the environment """ - client: BlueapiRestClient = obj["rest_client"] + assert isinstance(client := obj["rest_client"], BlueapiRestClient) if not reload: pprint(client.get_environment()) return diff --git a/src/blueapi/cli/rest.py b/src/blueapi/cli/rest.py index 46b88fbd3..df590c3dc 100644 --- a/src/blueapi/cli/rest.py +++ b/src/blueapi/cli/rest.py @@ -136,7 +136,7 @@ def _url(self, suffix: str) -> str: def get_environment(self) -> EnvironmentResponse: return self._request_and_deserialize("/environment", EnvironmentResponse) - def reload_environemnt(self) -> EnvironmentResponse: + def reload_environment(self) -> EnvironmentResponse: return self._request_and_deserialize( "/environment", EnvironmentResponse, method="DELETE" ) diff --git a/tests/test_cli.py b/tests/test_cli.py index 286c202ca..85235ca07 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,5 +1,5 @@ from dataclasses import dataclass -from unittest.mock import Mock, patch +from unittest.mock import MagicMock, Mock, patch import pytest from click.testing import CliRunner @@ -260,8 +260,9 @@ def test_reset_env( @pytest.mark.handler @patch("blueapi.service.handler.Handler") @patch("requests.request") -@patch("time.sleep", return_value=None) +@patch("blueapi.cli.cli.sleep", return_value=None) def test_reset_env2( + mock_sleep: MagicMock, mock_requests: Mock, mock_handler: Mock, handler: Handler, From 41ee94cb4022f9b41855ef28c0ece108bf4c3f08 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Thu, 25 Apr 2024 11:52:43 +0100 Subject: [PATCH 10/38] minor formatting changes --- src/blueapi/cli/cli.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index e81c416ca..b8cdb1820 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -5,7 +5,6 @@ from pathlib import Path from pprint import pprint from time import sleep -from typing import Optional import click from pydantic import ValidationError @@ -291,7 +290,7 @@ def stop(obj: dict) -> None: default=False, ) @click.pass_obj -def env(obj: dict, reload: Optional[bool]) -> None: +def env(obj: dict, reload: bool | None) -> None: """ Inspect or restart the environment """ From 89fcced7d4a66d150f6fe1f1c5df000f95c4dbd8 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Fri, 26 Apr 2024 12:19:12 +0000 Subject: [PATCH 11/38] fix B008 issue default args --- src/blueapi/service/main.py | 7 ++++--- tests/worker/test_reworker.py | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index cd4112912..4688f61c0 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -133,6 +133,9 @@ def get_device_by_name(name: str, handler: BlueskyHandler = Depends(get_handler) return handler.get_device(name) +default_task = Task(name="count", params={"detectors": ["x"]}) + + @app.post( "/tasks", response_model=TaskResponse, @@ -141,9 +144,7 @@ def get_device_by_name(name: str, handler: BlueskyHandler = Depends(get_handler) def submit_task( request: Request, response: Response, - task: Task = Body( - ..., example=Task(name="count", params={"detectors": ["x"]}) - ), # noqa: B008 + task: Task = Body(..., example=default_task), handler: BlueskyHandler = Depends(get_handler), ): """Submit a task to the worker.""" diff --git a/tests/worker/test_reworker.py b/tests/worker/test_reworker.py index 0c8ab2495..b23bda88b 100644 --- a/tests/worker/test_reworker.py +++ b/tests/worker/test_reworker.py @@ -346,11 +346,12 @@ def test_worker_and_data_events_produce_in_order(worker: Worker) -> None: def assert_running_count_plan_produces_ordered_worker_and_data_events( expected_events: list[WorkerEvent | DataEvent], worker: Worker, - task: Task = Task( - name="count", params={"detectors": ["image_det"], "num": 1} - ), # noqa: B008 + task: Task | None = None, timeout: float = 5.0, ) -> None: + default_task = Task(name="count", params={"detectors": ["image_det"], "num": 1}) + task = task or default_task + event_streams: list[EventStream[Any, int]] = [ worker.data_events, worker.worker_events, From 2dd5ec03cadbeacef4b31a810ea61a9295a08d4e Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Thu, 2 May 2024 15:16:55 +0100 Subject: [PATCH 12/38] update the return line in the rest client --- src/blueapi/cli/rest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/blueapi/cli/rest.py b/src/blueapi/cli/rest.py index df590c3dc..a73f1cb79 100644 --- a/src/blueapi/cli/rest.py +++ b/src/blueapi/cli/rest.py @@ -114,7 +114,7 @@ def _request_and_deserialize( data: Mapping[str, Any] | None = None, method="GET", raise_if: Callable[[requests.Response], bool] = _is_exception, - ) -> T: + ) -> T | BlueskyRemoteError: url = self._url(suffix) if data: response = requests.request(method, url, json=data) From d829595e37597372e13a4bdb4a49bb76eb32b337 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Mon, 13 May 2024 18:01:07 +0100 Subject: [PATCH 13/38] chang task into example task --- src/blueapi/service/main.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 4688f61c0..ed39b61c5 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -133,7 +133,7 @@ def get_device_by_name(name: str, handler: BlueskyHandler = Depends(get_handler) return handler.get_device(name) -default_task = Task(name="count", params={"detectors": ["x"]}) +example_task = Task(name="count", params={"detectors": ["x"]}) @app.post( @@ -144,7 +144,7 @@ def get_device_by_name(name: str, handler: BlueskyHandler = Depends(get_handler) def submit_task( request: Request, response: Response, - task: Task = Body(..., example=default_task), + task: Task = Body(..., example=example_task), handler: BlueskyHandler = Depends(get_handler), ): """Submit a task to the worker.""" From 2a862c4265e11ca11795a05080284ea6b2b1f560 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Mon, 13 May 2024 18:04:02 +0100 Subject: [PATCH 14/38] follow suggestions for error handling --- src/blueapi/cli/cli.py | 7 ++++++- src/blueapi/cli/rest.py | 8 ++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index b8cdb1820..898bca236 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -302,7 +302,12 @@ def env(obj: dict, reload: bool | None) -> None: # Reload the environment if needed print("Reloading the environment...") - pprint(client.reload_environment()) + try: + pprint(client.reload_environment()) + + except BlueskyRemoteError: + pprint("Failed to reload the environment") + exit() # Initialize a variable to keep track of the environment status environment_initialized = False diff --git a/src/blueapi/cli/rest.py b/src/blueapi/cli/rest.py index a73f1cb79..9be4fd4c2 100644 --- a/src/blueapi/cli/rest.py +++ b/src/blueapi/cli/rest.py @@ -114,18 +114,14 @@ def _request_and_deserialize( data: Mapping[str, Any] | None = None, method="GET", raise_if: Callable[[requests.Response], bool] = _is_exception, - ) -> T | BlueskyRemoteError: + ) -> T: url = self._url(suffix) if data: response = requests.request(method, url, json=data) else: response = requests.request(method, url) if raise_if(response): - message = get_status_message(response.status_code) - error_message = f"""Response failed with text: {response.text}, - with error code: {response.status_code} - which corresponds to {message}""" - raise BlueskyRemoteError(error_message) + raise BlueskyRemoteError(str(response)) deserialized = parse_obj_as(target_type, response.json()) return deserialized From 0e9d60ad9f3509110fbb39a58ffff2e781150e0b Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Mon, 13 May 2024 18:05:34 +0100 Subject: [PATCH 15/38] fix as suggested --- .vscode/launch.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 24d07cab7..4fa19aff6 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -28,8 +28,7 @@ "console": "integratedTerminal", "env": { // Enable break on exception when debugging tests (see: tests/conftest.py) - "PYTEST_RAISE": "1", - "PYTEST_ADDOPTS": "--no-cov -vv" + "PYTEST_RAISE": "1" }, }, { From 9f35b3f67ada99dbeaee878be37cfa8564adc6ef Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Wed, 15 May 2024 10:35:09 +0000 Subject: [PATCH 16/38] revert settings change --- .vscode/launch.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 4fa19aff6..a82bb594c 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -28,7 +28,7 @@ "console": "integratedTerminal", "env": { // Enable break on exception when debugging tests (see: tests/conftest.py) - "PYTEST_RAISE": "1" + "PYTEST_RAISE": "1", }, }, { @@ -78,4 +78,4 @@ ] } ] -} \ No newline at end of file +} From 0854b0f4dffb9a39001a572aba5e4e2b159f30de Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Wed, 15 May 2024 10:36:48 +0000 Subject: [PATCH 17/38] vscode settings restore --- .vscode/settings.json | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 81aac8ea9..e0e7cd387 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,19 +1,11 @@ { - "python.linting.pylintEnabled": false, - "python.linting.flake8Enabled": true, - "python.linting.mypyEnabled": true, - "python.linting.enabled": true, - "python.testing.pytestArgs": [ - "tests" - ], "python.testing.unittestEnabled": false, "python.testing.pytestEnabled": true, - "python.formatting.provider": "black", - "python.languageServer": "Pylance", "editor.formatOnSave": true, "editor.codeActionsOnSave": { "source.organizeImports": "explicit" }, - "esbonio.server.enabled": true, - "esbonio.sphinx.confDir": "" + "[python]": { + "editor.defaultFormatter": "charliermarsh.ruff", + }, } \ No newline at end of file From 8dc6cbc85f1f99cff5b975da1b2a7f35d8cae9ef Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Wed, 15 May 2024 12:28:07 +0000 Subject: [PATCH 18/38] revert formatting changes --- .vscode/settings.json | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index e0e7cd387..7820107a7 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,11 +1,11 @@ { - "python.testing.unittestEnabled": false, - "python.testing.pytestEnabled": true, - "editor.formatOnSave": true, - "editor.codeActionsOnSave": { - "source.organizeImports": "explicit" - }, - "[python]": { - "editor.defaultFormatter": "charliermarsh.ruff", - }, + "python.testing.unittestEnabled": false, + "python.testing.pytestEnabled": true, + "editor.formatOnSave": true, + "editor.codeActionsOnSave": { + "source.organizeImports": "explicit" + }, + "[python]": { + "editor.defaultFormatter": "charliermarsh.ruff", + }, } \ No newline at end of file From 4321cd54c51aacf35f6e809f51f38f4a660e529e Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Wed, 15 May 2024 16:31:58 +0100 Subject: [PATCH 19/38] settings corrected --- .vscode/settings.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 7820107a7..c129d991b 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,5 +1,5 @@ { - "python.testing.unittestEnabled": false, + "python.testing.unittestEnabled": false, "python.testing.pytestEnabled": true, "editor.formatOnSave": true, "editor.codeActionsOnSave": { From 959e8b0ba3e04846218637f5cd5ce6938edb5abc Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Fri, 17 May 2024 12:56:09 +0000 Subject: [PATCH 20/38] fix import --- tests/test_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 85235ca07..4101993c7 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,5 +1,5 @@ from dataclasses import dataclass -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import Mock, patch import pytest from click.testing import CliRunner From 9cd2f9086ba5d2c8bff514612183f28c72520465 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Thu, 23 May 2024 13:13:48 +0000 Subject: [PATCH 21/38] make lint happy --- tests/test_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 4101993c7..85235ca07 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,5 +1,5 @@ from dataclasses import dataclass -from unittest.mock import Mock, patch +from unittest.mock import MagicMock, Mock, patch import pytest from click.testing import CliRunner From ffc7412fe151347a0734b961c5267a14ce6c33e0 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Thu, 23 May 2024 13:24:21 +0000 Subject: [PATCH 22/38] removed all 'raise' from the cli' --- src/blueapi/cli/cli.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 898bca236..bb224325f 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -45,7 +45,8 @@ def main(ctx: click.Context, config: Path | None | tuple[Path, ...]) -> None: if path.exists(): config_loader.use_values_from_yaml(path) else: - raise FileNotFoundError(f"Cannot find file: {path}") + print(f"Cannot find file: {path}") + return ctx.ensure_object(dict) loaded_config: ApplicationConfig = config_loader.load() @@ -141,7 +142,8 @@ def listen_to_events(obj: dict) -> None: StompMessagingTemplate.autoconfigured(config.stomp) ) else: - raise RuntimeError("Message bus needs to be configured") + print("Message bus needs to be configured") + return def on_event( context: MessageContext, @@ -327,7 +329,8 @@ def env(obj: dict, reload: bool | None) -> None: polling_count += 1 sleep(1) # Wait for 1 seconds before checking again if polling_count == max_polling_count: - raise TimeoutError("Environment initialization timed out.") + print("Environment initialization timed out.") + return # Once out of the loop, print the initialized environment status pprint(environment_status) From 0beec9ceba52bba980659cef437575694f4f302e Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Thu, 23 May 2024 13:57:53 +0000 Subject: [PATCH 23/38] change more into logging not print --- src/blueapi/cli/cli.py | 50 ++++++++++++++++++++++-------------------- tests/test_cli.py | 3 +-- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index bb224325f..21dd45b32 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -3,7 +3,6 @@ from collections import deque from functools import wraps from pathlib import Path -from pprint import pprint from time import sleep import click @@ -45,7 +44,7 @@ def main(ctx: click.Context, config: Path | None | tuple[Path, ...]) -> None: if path.exists(): config_loader.use_values_from_yaml(path) else: - print(f"Cannot find file: {path}") + logging.error(f"Cannot find file: {path}") return ctx.ensure_object(dict) @@ -55,7 +54,7 @@ def main(ctx: click.Context, config: Path | None | tuple[Path, ...]) -> None: logging.basicConfig(level=loaded_config.logging.level) if ctx.invoked_subcommand is None: - print("Please invoke subcommand!") + logging.logger("Please invoke subcommand!") @main.command(name="schema") @@ -94,7 +93,7 @@ def controller(ctx: click.Context) -> None: """Client utility for controlling and introspecting the worker""" if ctx.invoked_subcommand is None: - print("Please invoke subcommand!") + logging.error("Please invoke subcommand!") return ctx.ensure_object(dict) @@ -108,7 +107,8 @@ def wrapper(*args, **kwargs): try: func(*args, **kwargs) except ConnectionError: - print("Failed to establish connection to FastAPI server.") + logging.error("Failed to establish connection to FastAPI server.") + return return wrapper @@ -119,7 +119,7 @@ def wrapper(*args, **kwargs): def get_plans(obj: dict) -> None: """Get a list of plans available for the worker to use""" client: BlueapiRestClient = obj["rest_client"] - pprint(client.get_plans().dict()) + logging.info(client.get_plans().dict()) @controller.command(name="devices") @@ -128,7 +128,7 @@ def get_plans(obj: dict) -> None: def get_devices(obj: dict) -> None: """Get a list of devices available for the worker to use""" client: BlueapiRestClient = obj["rest_client"] - pprint(client.get_devices().dict()) + logging.info(client.get_devices().dict()) @controller.command(name="listen") @@ -142,7 +142,7 @@ def listen_to_events(obj: dict) -> None: StompMessagingTemplate.autoconfigured(config.stomp) ) else: - print("Message bus needs to be configured") + logging.logger("Message bus needs to be configured") return def on_event( @@ -150,9 +150,9 @@ def on_event( event: WorkerEvent | ProgressEvent | DataEvent, ) -> None: converted = json.dumps(event.dict(), indent=2) - print(converted) + logging.logger(converted) - print( + logging.logger( "Subscribing to all bluesky events from " f"{config.stomp.host}:{config.stomp.port}" ) @@ -221,7 +221,7 @@ def store_finished_event(event: WorkerEvent) -> None: return process_event_after_finished(finished_event.pop(), logger) - pprint(updated.dict()) + logging.logger(updated.dict()) @controller.command(name="state") @@ -231,7 +231,7 @@ def get_state(obj: dict) -> None: """Print the current state of the worker""" client: BlueapiRestClient = obj["rest_client"] - pprint(client.get_state()) + logging.logger(client.get_state()) @controller.command(name="pause") @@ -242,7 +242,7 @@ def pause(obj: dict, defer: bool = False) -> None: """Pause the execution of the current task""" client: BlueapiRestClient = obj["rest_client"] - pprint(client.set_state(WorkerState.PAUSED, defer=defer)) + logging.logger(client.set_state(WorkerState.PAUSED, defer=defer)) @controller.command(name="resume") @@ -252,7 +252,7 @@ def resume(obj: dict) -> None: """Resume the execution of the current task""" client: BlueapiRestClient = obj["rest_client"] - pprint(client.set_state(WorkerState.RUNNING)) + logging.logger(client.set_state(WorkerState.RUNNING)) @controller.command(name="abort") @@ -266,7 +266,9 @@ def abort(obj: dict, reason: str | None = None) -> None: """ client: BlueapiRestClient = obj["rest_client"] - pprint(client.cancel_current_task(state=WorkerState.ABORTING, reason=reason)) + logging.logger( + client.cancel_current_task(state=WorkerState.ABORTING, reason=reason) + ) @controller.command(name="stop") @@ -278,7 +280,7 @@ def stop(obj: dict) -> None: """ client: BlueapiRestClient = obj["rest_client"] - pprint(client.cancel_current_task(state=WorkerState.STOPPING)) + logging.logger(client.cancel_current_task(state=WorkerState.STOPPING)) @controller.command(name="env") @@ -299,16 +301,16 @@ def env(obj: dict, reload: bool | None) -> None: assert isinstance(client := obj["rest_client"], BlueapiRestClient) if not reload: - pprint(client.get_environment()) + logging.logger(client.get_environment()) return # Reload the environment if needed - print("Reloading the environment...") + logging.logger("Reloading the environment...") try: - pprint(client.reload_environment()) + logging.logger(client.reload_environment()) except BlueskyRemoteError: - pprint("Failed to reload the environment") + logging.logger("Failed to reload the environment") exit() # Initialize a variable to keep track of the environment status @@ -322,18 +324,18 @@ def env(obj: dict, reload: bool | None) -> None: # Check if the environment is initialized if environment_status.initialized: - print("Environment is initialized.") + logging.info("Environment is initialized.") environment_initialized = True else: - print("Waiting for environment to initialize...") + logging.info("Waiting for environment to initialize...") polling_count += 1 sleep(1) # Wait for 1 seconds before checking again if polling_count == max_polling_count: - print("Environment initialization timed out.") + logging.error("Environment initialization timed out.") return # Once out of the loop, print the initialized environment status - pprint(environment_status) + logging.info(environment_status) # helper function diff --git a/tests/test_cli.py b/tests/test_cli.py index 85235ca07..bc82ad1d2 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -43,8 +43,7 @@ def test_main_with_nonexistent_config_file(): runner = CliRunner() result = runner.invoke(main, ["-c", "tests/non_existent.yaml"]) - assert result.exit_code == 1 - assert type(result.exception) is FileNotFoundError + assert result.exit_code == 0 @patch("requests.request") From 838cd985b2c7446653a128eb647a9ef7613ec563 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Tue, 28 May 2024 16:31:05 +0100 Subject: [PATCH 24/38] remove logs from cli --- src/blueapi/cli/cli.py | 48 ++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 21dd45b32..4a0cf2704 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -44,7 +44,7 @@ def main(ctx: click.Context, config: Path | None | tuple[Path, ...]) -> None: if path.exists(): config_loader.use_values_from_yaml(path) else: - logging.error(f"Cannot find file: {path}") + print(f"Cannot find file: {path}") return ctx.ensure_object(dict) @@ -54,7 +54,7 @@ def main(ctx: click.Context, config: Path | None | tuple[Path, ...]) -> None: logging.basicConfig(level=loaded_config.logging.level) if ctx.invoked_subcommand is None: - logging.logger("Please invoke subcommand!") + print("Please invoke subcommand!") @main.command(name="schema") @@ -93,7 +93,7 @@ def controller(ctx: click.Context) -> None: """Client utility for controlling and introspecting the worker""" if ctx.invoked_subcommand is None: - logging.error("Please invoke subcommand!") + print("Please invoke subcommand!") return ctx.ensure_object(dict) @@ -107,7 +107,7 @@ def wrapper(*args, **kwargs): try: func(*args, **kwargs) except ConnectionError: - logging.error("Failed to establish connection to FastAPI server.") + print("Failed to establish connection to FastAPI server.") return return wrapper @@ -119,7 +119,7 @@ def wrapper(*args, **kwargs): def get_plans(obj: dict) -> None: """Get a list of plans available for the worker to use""" client: BlueapiRestClient = obj["rest_client"] - logging.info(client.get_plans().dict()) + print(client.get_plans().dict()) @controller.command(name="devices") @@ -128,7 +128,7 @@ def get_plans(obj: dict) -> None: def get_devices(obj: dict) -> None: """Get a list of devices available for the worker to use""" client: BlueapiRestClient = obj["rest_client"] - logging.info(client.get_devices().dict()) + print(client.get_devices().dict()) @controller.command(name="listen") @@ -142,7 +142,7 @@ def listen_to_events(obj: dict) -> None: StompMessagingTemplate.autoconfigured(config.stomp) ) else: - logging.logger("Message bus needs to be configured") + print("Message bus needs to be configured") return def on_event( @@ -150,9 +150,9 @@ def on_event( event: WorkerEvent | ProgressEvent | DataEvent, ) -> None: converted = json.dumps(event.dict(), indent=2) - logging.logger(converted) + print(converted) - logging.logger( + print( "Subscribing to all bluesky events from " f"{config.stomp.host}:{config.stomp.port}" ) @@ -221,7 +221,7 @@ def store_finished_event(event: WorkerEvent) -> None: return process_event_after_finished(finished_event.pop(), logger) - logging.logger(updated.dict()) + print(updated.dict()) @controller.command(name="state") @@ -231,7 +231,7 @@ def get_state(obj: dict) -> None: """Print the current state of the worker""" client: BlueapiRestClient = obj["rest_client"] - logging.logger(client.get_state()) + print(client.get_state()) @controller.command(name="pause") @@ -242,7 +242,7 @@ def pause(obj: dict, defer: bool = False) -> None: """Pause the execution of the current task""" client: BlueapiRestClient = obj["rest_client"] - logging.logger(client.set_state(WorkerState.PAUSED, defer=defer)) + print(client.set_state(WorkerState.PAUSED, defer=defer)) @controller.command(name="resume") @@ -252,7 +252,7 @@ def resume(obj: dict) -> None: """Resume the execution of the current task""" client: BlueapiRestClient = obj["rest_client"] - logging.logger(client.set_state(WorkerState.RUNNING)) + print(client.set_state(WorkerState.RUNNING)) @controller.command(name="abort") @@ -266,9 +266,7 @@ def abort(obj: dict, reason: str | None = None) -> None: """ client: BlueapiRestClient = obj["rest_client"] - logging.logger( - client.cancel_current_task(state=WorkerState.ABORTING, reason=reason) - ) + print(client.cancel_current_task(state=WorkerState.ABORTING, reason=reason)) @controller.command(name="stop") @@ -280,7 +278,7 @@ def stop(obj: dict) -> None: """ client: BlueapiRestClient = obj["rest_client"] - logging.logger(client.cancel_current_task(state=WorkerState.STOPPING)) + print(client.cancel_current_task(state=WorkerState.STOPPING)) @controller.command(name="env") @@ -301,16 +299,16 @@ def env(obj: dict, reload: bool | None) -> None: assert isinstance(client := obj["rest_client"], BlueapiRestClient) if not reload: - logging.logger(client.get_environment()) + print(client.get_environment()) return # Reload the environment if needed - logging.logger("Reloading the environment...") + print("Reloading the environment...") try: - logging.logger(client.reload_environment()) + print(client.reload_environment()) except BlueskyRemoteError: - logging.logger("Failed to reload the environment") + print("Failed to reload the environment") exit() # Initialize a variable to keep track of the environment status @@ -324,18 +322,18 @@ def env(obj: dict, reload: bool | None) -> None: # Check if the environment is initialized if environment_status.initialized: - logging.info("Environment is initialized.") + print("Environment is initialized.") environment_initialized = True else: - logging.info("Waiting for environment to initialize...") + print("Waiting for environment to initialize...") polling_count += 1 sleep(1) # Wait for 1 seconds before checking again if polling_count == max_polling_count: - logging.error("Environment initialization timed out.") + print("Environment initialization timed out.") return # Once out of the loop, print the initialized environment status - logging.info(environment_status) + print(environment_status) # helper function From 3ec47fb711d4894dd5cf9eaf6c24a3efee190949 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Thu, 30 May 2024 08:51:02 +0100 Subject: [PATCH 25/38] move to pprint --- src/blueapi/cli/cli.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 4a0cf2704..7b0c9841a 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -3,6 +3,7 @@ from collections import deque from functools import wraps from pathlib import Path +from pprint import pprint from time import sleep import click @@ -119,7 +120,7 @@ def wrapper(*args, **kwargs): def get_plans(obj: dict) -> None: """Get a list of plans available for the worker to use""" client: BlueapiRestClient = obj["rest_client"] - print(client.get_plans().dict()) + pprint(client.get_plans().dict()) @controller.command(name="devices") @@ -128,7 +129,7 @@ def get_plans(obj: dict) -> None: def get_devices(obj: dict) -> None: """Get a list of devices available for the worker to use""" client: BlueapiRestClient = obj["rest_client"] - print(client.get_devices().dict()) + pprint(client.get_devices().dict()) @controller.command(name="listen") @@ -150,7 +151,7 @@ def on_event( event: WorkerEvent | ProgressEvent | DataEvent, ) -> None: converted = json.dumps(event.dict(), indent=2) - print(converted) + pprint(converted) print( "Subscribing to all bluesky events from " @@ -221,7 +222,7 @@ def store_finished_event(event: WorkerEvent) -> None: return process_event_after_finished(finished_event.pop(), logger) - print(updated.dict()) + pprint(updated.dict()) @controller.command(name="state") From a6a8d6f1091d840a9c72122332ffad56d5f4accc Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Thu, 30 May 2024 08:22:02 +0000 Subject: [PATCH 26/38] change the tests for a nicer set of commit messages --- src/blueapi/cli/cli.py | 5 +++-- tests/test_cli.py | 13 ++++--------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 7b0c9841a..2f5622d51 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -1,5 +1,6 @@ import json import logging +import sys from collections import deque from functools import wraps from pathlib import Path @@ -46,7 +47,7 @@ def main(ctx: click.Context, config: Path | None | tuple[Path, ...]) -> None: config_loader.use_values_from_yaml(path) else: print(f"Cannot find file: {path}") - return + sys.exit(1) ctx.ensure_object(dict) loaded_config: ApplicationConfig = config_loader.load() @@ -144,7 +145,7 @@ def listen_to_events(obj: dict) -> None: ) else: print("Message bus needs to be configured") - return + sys.exit(1) def on_event( context: MessageContext, diff --git a/tests/test_cli.py b/tests/test_cli.py index bc82ad1d2..3c185f535 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -132,6 +132,7 @@ def test_get_plans_and_devices( def test_invalid_config_path_handling(runner: CliRunner): # test what happens if you pass an invalid config file... result = runner.invoke(main, ["-c", "non_existent.yaml"]) + assert result.output == "Cannot find file: non_existent.yaml\n" assert result.exit_code == 1 @@ -172,14 +173,13 @@ def test_config_passed_down_to_command_children( def test_invalid_stomp_config_for_listener(runner: CliRunner): result = runner.invoke(main, ["controller", "listen"]) - assert ( - isinstance(result.exception, RuntimeError) - and str(result.exception) == "Message bus needs to be configured" - ) + assert result.exit_code == 1 + assert result.output == "Message bus needs to be configured\n" def test_cannot_run_plans_without_stomp_config(runner: CliRunner): result = runner.invoke(main, ["controller", "run", "sleep", '{"time": 5}']) + assert result.exit_code == 1 assert ( "Cannot run plans without Stomp configuration to track progress" in result.output @@ -201,11 +201,6 @@ def test_valid_stomp_config_for_listener(runner: CliRunner): assert result.exit_code == 0 -def test_invalid_condition_for_run(runner: CliRunner): - result = runner.invoke(main, ["controller", "run", "sleep", '{"time": 5}']) - assert type(result.exception) is RuntimeError - - @pytest.mark.handler @patch("blueapi.service.handler.Handler") @patch("requests.request") From b902ac9cf19ee2e7be1f6f77823df38303e89288 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Thu, 30 May 2024 09:26:16 +0100 Subject: [PATCH 27/38] remove redundant test --- tests/test_cli.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 3c185f535..7e68ff698 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -39,13 +39,6 @@ def test_main_no_params(): assert result.stdout == expected -def test_main_with_nonexistent_config_file(): - runner = CliRunner() - result = runner.invoke(main, ["-c", "tests/non_existent.yaml"]) - - assert result.exit_code == 0 - - @patch("requests.request") def test_connection_error_caught_by_wrapper_func(mock_requests: Mock): mock_requests.side_effect = ConnectionError() From 6fb6a7a504ddec170eaa70f58c8ced77b3a70b69 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Thu, 30 May 2024 14:13:21 +0000 Subject: [PATCH 28/38] finally fix the test to query the right endpoints --- pyproject.toml | 1 + src/blueapi/cli/cli.py | 8 ++-- tests/test_cli.py | 102 +++++++++++++++++++++++++++++------------ 3 files changed, 78 insertions(+), 33 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 7304ec3b6..f2405f3e5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,6 +43,7 @@ dev = [ "mypy", "pytest-cov", "pytest-asyncio", + "responses", "ruff", "sphinx-autobuild==2024.2.4", # Later versions have a clash with fastapi<0.99, remove pin when fastapi is a higher version "sphinx-copybutton", diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 2f5622d51..7d329f420 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -302,16 +302,16 @@ def env(obj: dict, reload: bool | None) -> None: assert isinstance(client := obj["rest_client"], BlueapiRestClient) if not reload: print(client.get_environment()) - return + exit() # Reload the environment if needed print("Reloading the environment...") try: - print(client.reload_environment()) + deserialized = client.reload_environment() + print(deserialized) except BlueskyRemoteError: - print("Failed to reload the environment") - exit() + exit("Failed to reload the environment") # Initialize a variable to keep track of the environment status environment_initialized = False diff --git a/tests/test_cli.py b/tests/test_cli.py index 7e68ff698..e0b398931 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -2,6 +2,7 @@ from unittest.mock import MagicMock, Mock, patch import pytest +import responses from click.testing import CliRunner from fastapi.testclient import TestClient from pydantic import BaseModel @@ -11,6 +12,7 @@ from blueapi.cli.cli import main from blueapi.core.bluesky_types import Plan from blueapi.service.handler import Handler, teardown_handler +from blueapi.service.model import EnvironmentResponse @pytest.fixture(autouse=True) @@ -221,53 +223,95 @@ def test_get_env( @pytest.mark.handler @patch("blueapi.service.handler.Handler") -@patch("requests.request") -def test_reset_env( - mock_requests: Mock, +@patch("blueapi.cli.rest.BlueapiRestClient.get_environment") +@patch("blueapi.cli.rest.BlueapiRestClient.reload_environment") +@patch("blueapi.cli.cli.sleep", return_value=None) +def test_reset_env_client_behavior( + mock_sleep: MagicMock, + mock_reload_environment: Mock, + mock_get_environment: Mock, mock_handler: Mock, handler: Handler, client: TestClient, runner: CliRunner, ): + # Configure the mock_requests to simulate different responses + # First two calls return not initialized, followed by an initialized response + mock_get_environment.side_effect = [ + EnvironmentResponse(initialized=False), # not initialized + EnvironmentResponse(initialized=False), # not initialized + EnvironmentResponse(initialized=True), # finally initalized + ] + mock_reload_environment.return_value = "Environment reload initiated." + with patch("uvicorn.run", side_effect=None): - result = runner.invoke(main, ["serve"]) + serve_result = runner.invoke(main, ["serve"]) - assert result.exit_code == 0 + assert serve_result.exit_code == 0 - mock_requests.return_value = Mock() + # Invoke the CLI command that would trigger the environment initialization check + reload_result = runner.invoke(main, ["controller", "env", "-r"]) - runner.invoke(main, ["controller", "env", "-r"]) + assert mock_get_environment.call_count == 3 - assert mock_requests.call_args[0] == ( - "DELETE", - "http://localhost:8000/environment", + # Verify if sleep was called between polling iterations + assert mock_sleep.call_count == 2 # Since the last check doesn't require a sleep + + # Check if the final environment status is printed correctly + # assert "Environment is initialized." in result.output + assert ( + reload_result.output + == "Reloading the environment...\nEnvironment reload initiated.\nWaiting for environment to initialize...\nWaiting for environment to initialize...\nEnvironment is initialized.\ninitialized=True\n" # noqa: E501 ) +@responses.activate @pytest.mark.handler @patch("blueapi.service.handler.Handler") -@patch("requests.request") @patch("blueapi.cli.cli.sleep", return_value=None) -def test_reset_env2( - mock_sleep: MagicMock, - mock_requests: Mock, - mock_handler: Mock, - handler: Handler, - client: TestClient, - runner: CliRunner, +def test_env_endpoint_interaction( + mock_sleep: MagicMock, mock_handler: Mock, handler: Handler, runner: CliRunner ): - with patch("uvicorn.run", side_effect=None): - result = runner.invoke(main, ["serve"]) + # Setup mocked responses for the REST endpoints + responses.add( + responses.DELETE, + "http://localhost:8000/environment", + status=200, + json=EnvironmentResponse(initialized=False).dict(), + ) + responses.add( + responses.GET, + "http://localhost:8000/environment", + json=EnvironmentResponse(initialized=False).dict(), + status=200, + ) + responses.add( + responses.GET, + "http://localhost:8000/environment", + json=EnvironmentResponse(initialized=False).dict(), + status=200, + ) + responses.add( + responses.GET, + "http://localhost:8000/environment", + status=200, + json=EnvironmentResponse(initialized=True).dict(), + ) - assert result.exit_code == 0 + # Run the command that should interact with these endpoints + result = runner.invoke(main, ["controller", "env", "-r"]) - mock_requests.return_value = Mock() + # Check if the endpoints were hit as expected + assert len(responses.calls) == 4 # Ensures that all expected calls were made - # Invoke the CLI command that would trigger the environment initialization check - runner.invoke(main, ["controller", "env", "-r"]) + for index, call in enumerate(responses.calls): + if index == 0: + assert call.request.method == "DELETE" + assert call.request.url == "http://localhost:8000/environment" + else: + assert call.request.method == "GET" + assert call.request.url == "http://localhost:8000/environment" - # Check if the DELETE request was made correctly - assert mock_requests.call_args[0] == ( - "DELETE", - "http://localhost:8000/environment", - ) + # Check other assertions as needed, e.g., output or exit codes + assert result.exit_code == 0 + assert "Environment is initialized." in result.output From 9408dc89a78156daba017ea09ec8a07f183b6b33 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Thu, 30 May 2024 14:30:23 +0000 Subject: [PATCH 29/38] make better coverage --- src/blueapi/cli/cli.py | 14 ++++++------- tests/test_cli.py | 47 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 7d329f420..3cb1d494d 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -152,7 +152,7 @@ def on_event( event: WorkerEvent | ProgressEvent | DataEvent, ) -> None: converted = json.dumps(event.dict(), indent=2) - pprint(converted) + print(converted) print( "Subscribing to all bluesky events from " @@ -233,7 +233,7 @@ def get_state(obj: dict) -> None: """Print the current state of the worker""" client: BlueapiRestClient = obj["rest_client"] - print(client.get_state()) + pprint(client.get_state()) @controller.command(name="pause") @@ -244,7 +244,7 @@ def pause(obj: dict, defer: bool = False) -> None: """Pause the execution of the current task""" client: BlueapiRestClient = obj["rest_client"] - print(client.set_state(WorkerState.PAUSED, defer=defer)) + pprint(client.set_state(WorkerState.PAUSED, defer=defer)) @controller.command(name="resume") @@ -254,7 +254,7 @@ def resume(obj: dict) -> None: """Resume the execution of the current task""" client: BlueapiRestClient = obj["rest_client"] - print(client.set_state(WorkerState.RUNNING)) + pprint(client.set_state(WorkerState.RUNNING)) @controller.command(name="abort") @@ -268,7 +268,7 @@ def abort(obj: dict, reason: str | None = None) -> None: """ client: BlueapiRestClient = obj["rest_client"] - print(client.cancel_current_task(state=WorkerState.ABORTING, reason=reason)) + pprint(client.cancel_current_task(state=WorkerState.ABORTING, reason=reason)) @controller.command(name="stop") @@ -302,7 +302,7 @@ def env(obj: dict, reload: bool | None) -> None: assert isinstance(client := obj["rest_client"], BlueapiRestClient) if not reload: print(client.get_environment()) - exit() + quit("Exiting the environment inspection") # Reload the environment if needed print("Reloading the environment...") @@ -311,7 +311,7 @@ def env(obj: dict, reload: bool | None) -> None: print(deserialized) except BlueskyRemoteError: - exit("Failed to reload the environment") + sys.stderr.write("Failed to reload the environment") # Initialize a variable to keep track of the environment status environment_initialized = False diff --git a/tests/test_cli.py b/tests/test_cli.py index e0b398931..199642303 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -315,3 +315,50 @@ def test_env_endpoint_interaction( # Check other assertions as needed, e.g., output or exit codes assert result.exit_code == 0 assert "Environment is initialized." in result.output + + +@pytest.mark.handler +@responses.activate +@patch("blueapi.service.handler.Handler") +@patch("blueapi.cli.cli.sleep", return_value=None) +def test_env_timeout( + mock_sleep: MagicMock, mock_handler: Mock, handler: Handler, runner: CliRunner +): + max_polling_count = 10 # Assuming this is your max polling count in the command + + # Setup mocked responses for the REST endpoints + responses.add( + responses.DELETE, + "http://localhost:8000/environment", + status=200, + json=EnvironmentResponse(initialized=False).dict(), + ) + # Add responses for each polling attempt, all indicating not initialized + for _ in range(max_polling_count): + responses.add( + responses.GET, + "http://localhost:8000/environment", + json=EnvironmentResponse(initialized=False).dict(), + status=200, + ) + + # Run the command that should interact with these endpoints + result = runner.invoke(main, ["controller", "env", "-r"]) + + # Check if the endpoints were hit as expected + assert len(responses.calls) == max_polling_count + 1 # +1 for the DELETE call + + # First call should be DELETE + assert responses.calls[0].request.method == "DELETE" + assert responses.calls[0].request.url == "http://localhost:8000/environment" + + # Remaining calls should all be GET + for call in responses.calls[1:]: # Skip the first DELETE request + assert call.request.method == "GET" + assert call.request.url == "http://localhost:8000/environment" + + # Check the output for the timeout message + assert "Environment initialization timed out." in result.output + assert ( + result.exit_code == 0 + ) # Assuming your command exits successfully even on timeout for simplicity From 94a7b05ab92188a8a8421a75f6568939d3689b83 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Fri, 31 May 2024 14:22:56 +0100 Subject: [PATCH 30/38] rollback raise changes --- src/blueapi/cli/cli.py | 18 ++++++++---------- tests/test_cli.py | 11 ++++++----- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 3cb1d494d..3c0af6e68 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -1,6 +1,5 @@ import json import logging -import sys from collections import deque from functools import wraps from pathlib import Path @@ -46,8 +45,7 @@ def main(ctx: click.Context, config: Path | None | tuple[Path, ...]) -> None: if path.exists(): config_loader.use_values_from_yaml(path) else: - print(f"Cannot find file: {path}") - sys.exit(1) + raise FileNotFoundError(f"Cannot find file: {path}") ctx.ensure_object(dict) loaded_config: ApplicationConfig = config_loader.load() @@ -110,7 +108,6 @@ def wrapper(*args, **kwargs): func(*args, **kwargs) except ConnectionError: print("Failed to establish connection to FastAPI server.") - return return wrapper @@ -144,8 +141,7 @@ def listen_to_events(obj: dict) -> None: StompMessagingTemplate.autoconfigured(config.stomp) ) else: - print("Message bus needs to be configured") - sys.exit(1) + raise RuntimeError("Message bus needs to be configured") def on_event( context: MessageContext, @@ -186,8 +182,9 @@ def run_plan( if config.stomp is not None: _message_template = StompMessagingTemplate.autoconfigured(config.stomp) else: - pprint("ERROR: Cannot run plans without Stomp configuration to track progress") - return + raise RuntimeError( + "Cannot run plans without Stomp configuration to track progress" + ) event_bus_client = EventBusClient(_message_template) finished_event: deque[WorkerEvent] = deque() @@ -280,7 +277,7 @@ def stop(obj: dict) -> None: """ client: BlueapiRestClient = obj["rest_client"] - print(client.cancel_current_task(state=WorkerState.STOPPING)) + pprint(client.cancel_current_task(state=WorkerState.STOPPING)) @controller.command(name="env") @@ -311,7 +308,8 @@ def env(obj: dict, reload: bool | None) -> None: print(deserialized) except BlueskyRemoteError: - sys.stderr.write("Failed to reload the environment") + pprint("Failed to reload the environment") + quit("Exiting the environment inspection") # Initialize a variable to keep track of the environment status environment_initialized = False diff --git a/tests/test_cli.py b/tests/test_cli.py index 199642303..5be26458d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -127,7 +127,6 @@ def test_get_plans_and_devices( def test_invalid_config_path_handling(runner: CliRunner): # test what happens if you pass an invalid config file... result = runner.invoke(main, ["-c", "non_existent.yaml"]) - assert result.output == "Cannot find file: non_existent.yaml\n" assert result.exit_code == 1 @@ -168,16 +167,18 @@ def test_config_passed_down_to_command_children( def test_invalid_stomp_config_for_listener(runner: CliRunner): result = runner.invoke(main, ["controller", "listen"]) - assert result.exit_code == 1 - assert result.output == "Message bus needs to be configured\n" + assert ( + isinstance(result.exception, RuntimeError) + and str(result.exception) == "Message bus needs to be configured" + ) def test_cannot_run_plans_without_stomp_config(runner: CliRunner): result = runner.invoke(main, ["controller", "run", "sleep", '{"time": 5}']) assert result.exit_code == 1 assert ( - "Cannot run plans without Stomp configuration to track progress" - in result.output + isinstance(result.exception, RuntimeError) + and str(result.exception) == "Message bus needs to be configured" ) From d13234ce0bfcb38b0288573316f2cd0e774366ca Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Mon, 3 Jun 2024 10:44:59 +0100 Subject: [PATCH 31/38] fix test expected message string --- tests/test_cli.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 5be26458d..bf44d8037 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -178,7 +178,8 @@ def test_cannot_run_plans_without_stomp_config(runner: CliRunner): assert result.exit_code == 1 assert ( isinstance(result.exception, RuntimeError) - and str(result.exception) == "Message bus needs to be configured" + and str(result.exception) + == "Cannot run plans without Stomp configuration to track progress" ) From 29d64bfcd092e80854eec5933cc677162c89e6fc Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Tue, 4 Jun 2024 06:50:41 +0000 Subject: [PATCH 32/38] add docs on env reload --- docs/tutorials/dev-run.md | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/docs/tutorials/dev-run.md b/docs/tutorials/dev-run.md index 8d60c0dd6..308253a96 100644 --- a/docs/tutorials/dev-run.md +++ b/docs/tutorials/dev-run.md @@ -1,19 +1,18 @@ -# Run/Debug in a Developer Environment +# Run/Debug in a Developer Environment Assuming you have setup a developer environment, you can run a development version of the bluesky worker. - ## Start Bluesky Worker Ensure you are inside your virtual environment: -``` + +``` source venv/bin/activate ``` +You will need to follow the instructions for setting up ActiveMQ as in [run cli instructions](../how-to/run-cli.md). -You will need to follow the instructions for setting up ActiveMQ as in [run cli instructions](../how-to/run-cli.md). - -The worker will be available from the command line (`blueapi serve`), but can be started from vscode with additional +The worker will be available from the command line (`blueapi serve`), but can be started from vscode with additional debugging capabilities. 1. Navigate to "Run and Debug" in the left hand menu. @@ -21,3 +20,9 @@ debugging capabilities. 3. Click the green "Run Button" [debug in vscode](../images/debug-vscode.png) + +## Develop devices + +When you select the 'scratch directory' option - where you have devices (dodal) and plans (BLxx-beamline) in a place like `/dls_sw/BLXX/software/blueapi/scratch`, then the list of devices available will refresh without interfacing with the K8S cluster. Just run the command `blueapi env -r` or `blueapi env --reload`. + +With this setup you get a developer loop: "write devices - write plans - test them with blueapi". From a1c232105aa1b520ec2735d4c1789bee60cfa717 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Tue, 4 Jun 2024 11:57:55 +0100 Subject: [PATCH 33/38] small changes to reload cli logic --- src/blueapi/cli/cli.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 3c0af6e68..be4bfe7e6 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -307,9 +307,8 @@ def env(obj: dict, reload: bool | None) -> None: deserialized = client.reload_environment() print(deserialized) - except BlueskyRemoteError: - pprint("Failed to reload the environment") - quit("Exiting the environment inspection") + except BlueskyRemoteError as e: + raise BlueskyRemoteError("Failed to reload the environment") from e # Initialize a variable to keep track of the environment status environment_initialized = False @@ -329,8 +328,7 @@ def env(obj: dict, reload: bool | None) -> None: polling_count += 1 sleep(1) # Wait for 1 seconds before checking again if polling_count == max_polling_count: - print("Environment initialization timed out.") - return + raise TimeoutError("Environment initialization timed out.") # Once out of the loop, print the initialized environment status print(environment_status) From 9ff48be4e1237bb939acab0dfdcc90298dcdd38f Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Tue, 11 Jun 2024 09:22:54 +0000 Subject: [PATCH 34/38] add pytest raises to the timout test --- tests/test_cli.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index bf44d8037..999a9a4c4 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -6,7 +6,7 @@ from click.testing import CliRunner from fastapi.testclient import TestClient from pydantic import BaseModel -from requests.exceptions import ConnectionError +from requests.exceptions import ConnectionError, TimeoutError from blueapi import __version__ from blueapi.cli.cli import main @@ -345,7 +345,8 @@ def test_env_timeout( ) # Run the command that should interact with these endpoints - result = runner.invoke(main, ["controller", "env", "-r"]) + with pytest.raises(TimeoutError) as e: + result = runner.invoke(main, ["controller", "env", "-r"]) # Check if the endpoints were hit as expected assert len(responses.calls) == max_polling_count + 1 # +1 for the DELETE call @@ -360,7 +361,7 @@ def test_env_timeout( assert call.request.url == "http://localhost:8000/environment" # Check the output for the timeout message - assert "Environment initialization timed out." in result.output + assert "Environment initialization timed out." in e.output assert ( result.exit_code == 0 ) # Assuming your command exits successfully even on timeout for simplicity From 41fca37a8c358d6ec6105dc1b4773063d5140334 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Tue, 11 Jun 2024 13:10:12 +0100 Subject: [PATCH 35/38] fix timeout test --- tests/test_cli.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 999a9a4c4..74801fe9d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -6,7 +6,7 @@ from click.testing import CliRunner from fastapi.testclient import TestClient from pydantic import BaseModel -from requests.exceptions import ConnectionError, TimeoutError +from requests.exceptions import ConnectionError from blueapi import __version__ from blueapi.cli.cli import main @@ -345,8 +345,12 @@ def test_env_timeout( ) # Run the command that should interact with these endpoints - with pytest.raises(TimeoutError) as e: - result = runner.invoke(main, ["controller", "env", "-r"]) + result = runner.invoke(main, ["controller", "env", "-r"]) + if result.exception is not None: + assert isinstance(result.exception, TimeoutError), "Expected a TimeoutError" + assert result.exception.args[0] == "Environment initialization timed out." + else: + raise AssertionError("Expected an exception but got None") # Check if the endpoints were hit as expected assert len(responses.calls) == max_polling_count + 1 # +1 for the DELETE call @@ -361,7 +365,6 @@ def test_env_timeout( assert call.request.url == "http://localhost:8000/environment" # Check the output for the timeout message - assert "Environment initialization timed out." in e.output assert ( - result.exit_code == 0 + result.exit_code == 1 ) # Assuming your command exits successfully even on timeout for simplicity From 4427cdbfdc26b178ad648959db7225953e2eb633 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Tue, 11 Jun 2024 16:50:05 +0100 Subject: [PATCH 36/38] remove 1 line to pass codecov --- src/blueapi/cli/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index be4bfe7e6..81538036a 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -299,7 +299,7 @@ def env(obj: dict, reload: bool | None) -> None: assert isinstance(client := obj["rest_client"], BlueapiRestClient) if not reload: print(client.get_environment()) - quit("Exiting the environment inspection") + return # Reload the environment if needed print("Reloading the environment...") From d0d9694f95c2220fd36e5283f61026f7d8b02f92 Mon Sep 17 00:00:00 2001 From: Callum Forrester Date: Wed, 12 Jun 2024 10:26:30 +0100 Subject: [PATCH 37/38] Add test for environment reload failure --- src/blueapi/cli/cli.py | 67 +++++++++++++++++++++--------------------- tests/test_cli.py | 38 ++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 34 deletions(-) diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 81538036a..d2553f500 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -297,41 +297,40 @@ def env(obj: dict, reload: bool | None) -> None: """ assert isinstance(client := obj["rest_client"], BlueapiRestClient) - if not reload: + if reload: + # Reload the environment if needed + print("Reloading the environment...") + try: + deserialized = client.reload_environment() + print(deserialized) + + except BlueskyRemoteError as e: + raise BlueskyRemoteError("Failed to reload the environment") from e + + # Initialize a variable to keep track of the environment status + environment_initialized = False + polling_count = 0 + max_polling_count = 10 + # Use a while loop to keep checking until the environment is initialized + while not environment_initialized and polling_count < max_polling_count: + # Fetch the current environment status + environment_status = client.get_environment() + + # Check if the environment is initialized + if environment_status.initialized: + print("Environment is initialized.") + environment_initialized = True + else: + print("Waiting for environment to initialize...") + polling_count += 1 + sleep(1) # Wait for 1 seconds before checking again + if polling_count == max_polling_count: + raise TimeoutError("Environment initialization timed out.") + + # Once out of the loop, print the initialized environment status + print(environment_status) + else: print(client.get_environment()) - return - - # Reload the environment if needed - print("Reloading the environment...") - try: - deserialized = client.reload_environment() - print(deserialized) - - except BlueskyRemoteError as e: - raise BlueskyRemoteError("Failed to reload the environment") from e - - # Initialize a variable to keep track of the environment status - environment_initialized = False - polling_count = 0 - max_polling_count = 10 - # Use a while loop to keep checking until the environment is initialized - while not environment_initialized and polling_count < max_polling_count: - # Fetch the current environment status - environment_status = client.get_environment() - - # Check if the environment is initialized - if environment_status.initialized: - print("Environment is initialized.") - environment_initialized = True - else: - print("Waiting for environment to initialize...") - polling_count += 1 - sleep(1) # Wait for 1 seconds before checking again - if polling_count == max_polling_count: - raise TimeoutError("Environment initialization timed out.") - - # Once out of the loop, print the initialized environment status - print(environment_status) # helper function diff --git a/tests/test_cli.py b/tests/test_cli.py index 74801fe9d..2c913767a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -10,6 +10,7 @@ from blueapi import __version__ from blueapi.cli.cli import main +from blueapi.cli.event_bus_client import BlueskyRemoteError from blueapi.core.bluesky_types import Plan from blueapi.service.handler import Handler, teardown_handler from blueapi.service.model import EnvironmentResponse @@ -368,3 +369,40 @@ def test_env_timeout( assert ( result.exit_code == 1 ) # Assuming your command exits successfully even on timeout for simplicity + + +@pytest.mark.handler +@responses.activate +@patch("blueapi.service.handler.Handler") +@patch("blueapi.cli.cli.sleep", return_value=None) +def test_env_reload_server_side_error( + mock_sleep: MagicMock, mock_handler: Mock, handler: Handler, runner: CliRunner +): + # Setup mocked error response from the server + responses.add( + responses.DELETE, + "http://localhost:8000/environment", + status=500, + json={}, + ) + # Run the command that should interact with these endpoints + result = runner.invoke(main, ["controller", "env", "-r"]) + if result.exception is not None: + assert isinstance( + result.exception, BlueskyRemoteError + ), "Expected a BlueskyRemoteError" + assert result.exception.args[0] == "Failed to reload the environment" + else: + raise AssertionError("Expected an exception but got None") + + # Check if the endpoints were hit as expected + assert len(responses.calls) == 1 # +1 for the DELETE call + + # Only call should be DELETE + assert responses.calls[0].request.method == "DELETE" + assert responses.calls[0].request.url == "http://localhost:8000/environment" + + # Check the output for the timeout message + assert ( + result.exit_code == 1 + ) # Assuming your command exits successfully even on timeout for simplicity From 665e39613642dcebbd20f9a8eb77f4df0ea88968 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Thu, 20 Jun 2024 14:44:45 +0100 Subject: [PATCH 38/38] fix lint --- tests/test_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 83922f6d0..34303c922 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -407,6 +407,7 @@ def test_env_reload_server_side_error( result.exit_code == 1 ) # Assuming your command exits successfully even on timeout for simplicity + @pytest.fixture def mock_config(): # Mock configuration setup @@ -440,4 +441,3 @@ def test_error_handling(mock_config, exception, expected_exit_code, runner: CliR obj=mock_config, ) assert result.exit_code == expected_exit_code -