From addd5cc0c6190041671606e065e1e8e58afd1000 Mon Sep 17 00:00:00 2001 From: Peter Krull Date: Mon, 23 Oct 2023 19:44:09 -0600 Subject: [PATCH 1/2] Make connecting to server more robust --- pyproject.toml | 2 +- src/ansys/additive/core/additive.py | 5 +- src/ansys/additive/core/server_utils.py | 32 +++++++++ .../test_parametric_runner.py | 21 +++--- .../parametric_study/test_parametric_study.py | 8 +-- tests/test_additive.py | 65 ++++++++++++++++++- tests/test_server_utils.py | 33 +++++++++- 7 files changed, 144 insertions(+), 22 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index d09ee3584..925c79ffb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ build-backend = "flit_core.buildapi" [project] # Check https://flit.readthedocs.io/en/latest/pyproject_toml.html for all available sections name = "ansys-additive-core" -version = "0.16.dev1" +version = "0.16.dev2" description = "A python client for the Ansys Additive service" readme = "README.rst" requires-python = ">=3.9,<4" diff --git a/src/ansys/additive/core/additive.py b/src/ansys/additive/core/additive.py index 38257c40f..dbc5cd355 100644 --- a/src/ansys/additive/core/additive.py +++ b/src/ansys/additive/core/additive.py @@ -51,7 +51,7 @@ import ansys.additive.core.misc as misc from ansys.additive.core.porosity import PorositySummary from ansys.additive.core.progress_logger import ProgressLogger -from ansys.additive.core.server_utils import find_open_port, launch_server +from ansys.additive.core.server_utils import find_open_port, launch_server, server_ready from ansys.additive.core.simulation import SimulationError from ansys.additive.core.single_bead import SingleBeadSummary from ansys.additive.core.thermal_history import ThermalHistoryInput, ThermalHistorySummary @@ -120,6 +120,9 @@ def __init__( self._simulation_stub = SimulationServiceStub(self._channel) self._about_stub = AboutServiceStub(self._channel) + if not server_ready(self._about_stub): + raise RuntimeError("Unable to connect to server") + # Setup data directory self._user_data_path = USER_DATA_PATH if not os.path.exists(self._user_data_path): # pragma: no cover diff --git a/src/ansys/additive/core/server_utils.py b/src/ansys/additive/core/server_utils.py index 8ca5e9e6b..f2d866ba3 100644 --- a/src/ansys/additive/core/server_utils.py +++ b/src/ansys/additive/core/server_utils.py @@ -25,6 +25,9 @@ import subprocess import time +from ansys.api.additive.v0.about_pb2_grpc import AboutServiceStub +from google.protobuf.empty_pb2 import Empty + from ansys.additive.core import USER_DATA_PATH DEFAULT_ANSYS_VERSION = "241" @@ -111,3 +114,32 @@ def find_open_port() -> int: port = s.getsockname()[1] s.close() return port + + +def server_ready(stub: AboutServiceStub, retries: int = 5) -> bool: + """Return whether the server is ready. + + Parameters + ---------- + stub: AboutServiceStub + Stub used to call the about service on the server. + retries: int + Number of times to retry before giving up. An exponential backoff delay + is used between each retry. + + Returns + ------- + bool: + True means server is ready. False means the number of retries was exceeded + without successfully connecting to the server. + """ + ready = False + for i in range(retries): + try: + stub.About(Empty()) + ready = True + break + except Exception: + time.sleep(i + 1) + + return ready diff --git a/tests/parametric_study/test_parametric_runner.py b/tests/parametric_study/test_parametric_runner.py index 27a619641..245405a3e 100644 --- a/tests/parametric_study/test_parametric_runner.py +++ b/tests/parametric_study/test_parametric_runner.py @@ -23,6 +23,7 @@ from unittest.mock import create_autospec import pandas as pd +import pytest from ansys.additive.core import ( AdditiveMachine, @@ -279,9 +280,9 @@ def test_create_microstructure_input_assigns_defaults_for_nans(): assert input.material == material -def test_simulate_sorts_by_priority(): +def test_simulate_sorts_by_priority(tmp_path: pytest.TempPathFactory): # arrange - study = ps.ParametricStudy(study_name="test_study") + study = ps.ParametricStudy("test_study", tmp_path) material = AdditiveMaterial(name="test_material") sb = SingleBeadInput(id="test_1", material=material) p = PorosityInput(id="test_2", material=material) @@ -300,9 +301,9 @@ def test_simulate_sorts_by_priority(): mock_additive.simulate.assert_called_once_with(inputs) -def test_simulate_filters_by_priority(): +def test_simulate_filters_by_priority(tmp_path: pytest.TempPathFactory): # arrange - study = ps.ParametricStudy(study_name="test_study") + study = ps.ParametricStudy("test_study", tmp_path) material = AdditiveMaterial(name="test_material") sb = SingleBeadInput(id="test_1", material=material) p = PorosityInput(id="test_2", material=material) @@ -321,9 +322,9 @@ def test_simulate_filters_by_priority(): mock_additive.simulate.assert_called_once_with(inputs) -def test_simulate_filters_by_single_simulation_type(): +def test_simulate_filters_by_single_simulation_type(tmp_path: pytest.TempPathFactory): # arrange - study = ps.ParametricStudy(study_name="test_study") + study = ps.ParametricStudy("test_study", tmp_path) material = AdditiveMaterial(name="test_material") sb = SingleBeadInput(id="test_1", material=material) p = PorosityInput(id="test_2", material=material) @@ -342,9 +343,9 @@ def test_simulate_filters_by_single_simulation_type(): mock_additive.simulate.assert_called_once_with(inputs) -def test_simulate_filters_by_simulation_type_list(): +def test_simulate_filters_by_simulation_type_list(tmp_path: pytest.TempPathFactory): # arrange - study = ps.ParametricStudy(study_name="test_study") + study = ps.ParametricStudy("test_study", tmp_path) material = AdditiveMaterial(name="test_material") sb = SingleBeadInput(id="test_1", material=material) p = PorosityInput(id="test_2", material=material) @@ -367,9 +368,9 @@ def test_simulate_filters_by_simulation_type_list(): mock_additive.simulate.assert_called_once_with(inputs) -def test_simulate_skips_simulations_with_missing_materials(): +def test_simulate_skips_simulations_with_missing_materials(tmp_path: pytest.TempPathFactory): # arrange - study = ps.ParametricStudy(study_name="test_study") + study = ps.ParametricStudy("test_study", tmp_path) material = AdditiveMaterial(name="test_material") sb = SingleBeadInput(id="test_1", material=material) p = PorosityInput(id="test_2", material=material) diff --git a/tests/parametric_study/test_parametric_study.py b/tests/parametric_study/test_parametric_study.py index 33aa2a4c2..2604c0201 100644 --- a/tests/parametric_study/test_parametric_study.py +++ b/tests/parametric_study/test_parametric_study.py @@ -20,7 +20,6 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. -import os import platform from unittest.mock import PropertyMock, create_autospec, patch import uuid @@ -113,7 +112,7 @@ def test_load_raises_exception_when_version_too_great(tmp_path: pytest.TempPathF ps.ParametricStudy.load(filename) -@pytest.mark.skipif(platform.system != "Windows", reason="Only runs on Windows.") +@pytest.mark.skipif(platform.system() != "Windows", reason=f"Only runs on Windows.") def test_load_reads_linux_file_on_windows(tmp_path: pytest.TempPathFactory): # arrange filename = test_utils.get_test_file_path("linux.ps") @@ -125,7 +124,7 @@ def test_load_reads_linux_file_on_windows(tmp_path: pytest.TempPathFactory): assert study != None -@pytest.mark.skipif(platform.system == "Windows", reason="Only runs on linux.") +@pytest.mark.skipif(platform.system() == "Windows", reason="Only runs on linux.") def test_load_reads_windows_file_on_linux(tmp_path: pytest.TempPathFactory): # arrange filename = test_utils.get_test_file_path("windows.ps") @@ -151,9 +150,6 @@ def test_save_and_load_returns_original_object(tmp_path: pytest.TempPathFactory) assert study.data_frame().equals(study2.data_frame()) assert study2.file_name == test_path - # cleanup - os.remove(f"{study_name}.ps") - def test_add_summaries_with_porosity_summary_adds_row(tmp_path: pytest.TempPathFactory): # arrange diff --git a/tests/test_additive.py b/tests/test_additive.py index ca72a4c19..58961048d 100644 --- a/tests/test_additive.py +++ b/tests/test_additive.py @@ -27,6 +27,7 @@ from ansys.api.additive import __version__ as api_version from ansys.api.additive.v0.about_pb2 import AboutResponse +import ansys.api.additive.v0.about_pb2_grpc from ansys.api.additive.v0.about_pb2_grpc import AboutServiceStub import ansys.platform.instancemanagement as pypim from callee import Contains @@ -59,6 +60,10 @@ def test_Additive_init_connects_with_defaults(monkeypatch): monkeypatch.setattr(ansys.additive.core.additive, "launch_server", mock_launcher) mock_insecure_channel = create_autospec(grpc.insecure_channel, return_value=channel) monkeypatch.setattr(grpc, "insecure_channel", mock_insecure_channel) + mock_server_ready = create_autospec( + ansys.additive.core.additive.server_ready, return_value=True + ) + monkeypatch.setattr(ansys.additive.core.additive, "server_ready", mock_server_ready) # act additive = Additive() @@ -100,6 +105,10 @@ def test_Additive_init_can_connect_with_pypim(monkeypatch): monkeypatch.setattr(pypim, "connect", mock_connect) monkeypatch.setattr(pypim, "is_configured", mock_is_configured) monkeypatch.setattr(grpc, "insecure_channel", mock_insecure_channel) + mock_server_ready = create_autospec( + ansys.additive.core.additive.server_ready, return_value=True + ) + monkeypatch.setattr(ansys.additive.core.additive, "server_ready", mock_server_ready) # act additive = Additive() @@ -132,6 +141,10 @@ def test_Additive_init_connects_using_ANSYS_ADDITIVE_ADDRESS_if_available(monkey ) mock_insecure_channel = create_autospec(grpc.insecure_channel, return_value=channel) monkeypatch.setattr(grpc, "insecure_channel", mock_insecure_channel) + mock_server_ready = create_autospec( + ansys.additive.core.additive.server_ready, return_value=True + ) + monkeypatch.setattr(ansys.additive.core.additive, "server_ready", mock_server_ready) # act additive = Additive() @@ -157,6 +170,10 @@ def test_Additive_init_connects_with_ip_and_port_parameters(monkeypatch): ) mock_insecure_channel = create_autospec(grpc.insecure_channel, return_value=channel) monkeypatch.setattr(grpc, "insecure_channel", mock_insecure_channel) + mock_server_ready = create_autospec( + ansys.additive.core.additive.server_ready, return_value=True + ) + monkeypatch.setattr(ansys.additive.core.additive, "server_ready", mock_server_ready) # act additive = Additive(host=ip, port=port) @@ -185,6 +202,10 @@ def test_Additive_init_converts_hostname_to_ip(monkeypatch): monkeypatch.setattr(grpc, "insecure_channel", mock_insecure_channel) mock_gethostbyname = create_autospec(socket.gethostbyname, return_value=ip) monkeypatch.setattr(socket, "gethostbyname", mock_gethostbyname) + mock_server_ready = create_autospec( + ansys.additive.core.additive.server_ready, return_value=True + ) + monkeypatch.setattr(ansys.additive.core.additive, "server_ready", mock_server_ready) # act additive = Additive(host=host, port=port) @@ -196,6 +217,29 @@ def test_Additive_init_converts_hostname_to_ip(monkeypatch): ) +def test_Additive_init_raises_exception_if_server_ready_false(monkeypatch): + # arrange + ip = "1.2.3.4" + port = 12345 + target = f"{ip}:{port}" + channel = grpc.insecure_channel( + "channel_str", + options=[ + ("grpc.max_receive_message_length", MAX_MESSAGE_LENGTH), + ], + ) + mock_insecure_channel = create_autospec(grpc.insecure_channel, return_value=channel) + monkeypatch.setattr(grpc, "insecure_channel", mock_insecure_channel) + mock_server_ready = create_autospec( + ansys.additive.core.additive.server_ready, return_value=False + ) + monkeypatch.setattr(ansys.additive.core.additive, "server_ready", mock_server_ready) + + # act, assert + with pytest.raises(RuntimeError, match="Unable to connect to server"): + Additive(host=ip, port=port) + + @pytest.mark.parametrize( "input", [ @@ -205,8 +249,13 @@ def test_Additive_init_converts_hostname_to_ip(monkeypatch): ThermalHistoryInput(), ], ) -def test_simulate_without_material_assigned_raises_exception(input): +def test_simulate_without_material_assigned_raises_exception(monkeypatch, input): # arrange + mock_server_ready = create_autospec( + ansys.additive.core.additive.server_ready, return_value=True + ) + monkeypatch.setattr(ansys.additive.core.additive, "server_ready", mock_server_ready) + additive = Additive(host="localhost", port=12345) # act, assert @@ -214,8 +263,13 @@ def test_simulate_without_material_assigned_raises_exception(input): additive.simulate(input) -def test_simulate_list_of_inputs_with_duplicate_ids_raises_exception(): +def test_simulate_list_of_inputs_with_duplicate_ids_raises_exception(monkeypatch): # arrange + mock_server_ready = create_autospec( + ansys.additive.core.additive.server_ready, return_value=True + ) + monkeypatch.setattr(ansys.additive.core.additive, "server_ready", mock_server_ready) + additive = Additive(host="localhost", port=12345) inputs = [ SingleBeadInput(id="id"), @@ -227,8 +281,13 @@ def test_simulate_list_of_inputs_with_duplicate_ids_raises_exception(): additive.simulate(inputs) -def test_about_returns_about_response(): +def test_about_returns_about_response(monkeypatch): # arrange + mock_server_ready = create_autospec( + ansys.additive.core.additive.server_ready, return_value=True + ) + monkeypatch.setattr(ansys.additive.core.additive, "server_ready", mock_server_ready) + def mock_about_endpoint(request: Empty): response = AboutResponse() response.metadata["key1"] = "value1" diff --git a/tests/test_server_utils.py b/tests/test_server_utils.py index 2775993a9..69dd0f0c1 100644 --- a/tests/test_server_utils.py +++ b/tests/test_server_utils.py @@ -27,9 +27,16 @@ import tempfile from unittest.mock import ANY, Mock, patch +from ansys.api.additive.v0.about_pb2_grpc import AboutServiceStub +import grpc import pytest -from ansys.additive.core.server_utils import DEFAULT_ANSYS_VERSION, find_open_port, launch_server +from ansys.additive.core.server_utils import ( + DEFAULT_ANSYS_VERSION, + find_open_port, + launch_server, + server_ready, +) @patch("os.name", "unknown_os") @@ -185,3 +192,27 @@ def test_find_open_port_returns_valid_port(): # assert assert port > 1024 and port < 65535 + + +def test_server_ready_returns_false_when_server_cannot_be_reached(): + # arrange + channel = grpc.insecure_channel("channel") + stub = AboutServiceStub(channel) + + # act + ready = server_ready(stub, 1) + + # assert + assert ready == False + + +@patch("ansys.api.additive.v0.about_pb2_grpc.AboutServiceStub") +def test_server_ready_returns_true_when_server_can_be_reached(mock_stub): + # arrange + mock_stub.About.return_value = "all about it" + + # act + ready = server_ready(mock_stub) + + # assert + assert ready == True From c52dd6ba817b3428d8bd1cca7bfd3fabcdd21461 Mon Sep 17 00:00:00 2001 From: Peter Krull Date: Tue, 24 Oct 2023 08:34:19 -0600 Subject: [PATCH 2/2] Update test skip reason --- tests/parametric_study/test_parametric_study.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/parametric_study/test_parametric_study.py b/tests/parametric_study/test_parametric_study.py index 2604c0201..9eda5d2ea 100644 --- a/tests/parametric_study/test_parametric_study.py +++ b/tests/parametric_study/test_parametric_study.py @@ -112,7 +112,7 @@ def test_load_raises_exception_when_version_too_great(tmp_path: pytest.TempPathF ps.ParametricStudy.load(filename) -@pytest.mark.skipif(platform.system() != "Windows", reason=f"Only runs on Windows.") +@pytest.mark.skipif(platform.system() != "Windows", reason="Test only valid on Windows.") def test_load_reads_linux_file_on_windows(tmp_path: pytest.TempPathFactory): # arrange filename = test_utils.get_test_file_path("linux.ps") @@ -124,7 +124,7 @@ def test_load_reads_linux_file_on_windows(tmp_path: pytest.TempPathFactory): assert study != None -@pytest.mark.skipif(platform.system() == "Windows", reason="Only runs on linux.") +@pytest.mark.skipif(platform.system() == "Windows", reason="Test only valid on Linux.") def test_load_reads_windows_file_on_linux(tmp_path: pytest.TempPathFactory): # arrange filename = test_utils.get_test_file_path("windows.ps")