Skip to content

Commit

Permalink
Make connecting to server more robust (#133)
Browse files Browse the repository at this point in the history
  • Loading branch information
pkrull-ansys authored Oct 24, 2023
1 parent d70d7ae commit 52e1bfd
Show file tree
Hide file tree
Showing 7 changed files with 144 additions and 22 deletions.
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 4 additions & 1 deletion src/ansys/additive/core/additive.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
32 changes: 32 additions & 0 deletions src/ansys/additive/core/server_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
21 changes: 11 additions & 10 deletions tests/parametric_study/test_parametric_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from unittest.mock import create_autospec

import pandas as pd
import pytest

from ansys.additive.core import (
AdditiveMachine,
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
8 changes: 2 additions & 6 deletions tests/parametric_study/test_parametric_study.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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="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")
Expand All @@ -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="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")
Expand All @@ -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
Expand Down
65 changes: 62 additions & 3 deletions tests/test_additive.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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",
[
Expand All @@ -205,17 +249,27 @@ 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
with pytest.raises(ValueError, match="Material must be specified"):
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"),
Expand All @@ -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"
Expand Down
33 changes: 32 additions & 1 deletion tests/test_server_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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

0 comments on commit 52e1bfd

Please sign in to comment.