From 9ec5243f4e6dc33232ad8306e9bf1d938aa854e5 Mon Sep 17 00:00:00 2001 From: mbattu Date: Tue, 8 Aug 2023 14:31:45 +0530 Subject: [PATCH 01/47] Feat: Added global boolean to identify test environment --- .../_internal/utilities/globaltestingstate.py | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 ni_measurementlink_service/_internal/utilities/globaltestingstate.py diff --git a/ni_measurementlink_service/_internal/utilities/globaltestingstate.py b/ni_measurementlink_service/_internal/utilities/globaltestingstate.py new file mode 100644 index 000000000..17e6d2610 --- /dev/null +++ b/ni_measurementlink_service/_internal/utilities/globaltestingstate.py @@ -0,0 +1,8 @@ +"""Contains indicators to identify the Production/Test environment. +""" + + +class GlobalTestingState: + """Indicator for measurement running in testing environment.""" + + IsInTestState: bool = False From eb74dd66376ed981ae5e20c59ce186e8f0c7fd0c Mon Sep 17 00:00:00 2001 From: mbattu Date: Tue, 8 Aug 2023 14:32:39 +0530 Subject: [PATCH 02/47] Fix: Setting IsInTestState boolean to true for measurements in tests --- tests/acceptance/test_measurement_service.py | 2 ++ tests/acceptance/test_streaming_data_measurement.py | 2 ++ tests/acceptance/test_yield_vs_return.py | 2 ++ 3 files changed, 6 insertions(+) diff --git a/tests/acceptance/test_measurement_service.py b/tests/acceptance/test_measurement_service.py index 2d9853cc6..7c9b757ea 100644 --- a/tests/acceptance/test_measurement_service.py +++ b/tests/acceptance/test_measurement_service.py @@ -17,6 +17,7 @@ measurement_service_pb2 as v2_measurement_service_pb2, measurement_service_pb2_grpc as v2_measurement_service_pb2_grpc, ) +from ni_measurementlink_service._internal.utilities.globaltestingstate import GlobalTestingState from ni_measurementlink_service.measurement.service import MeasurementService from tests.assets import sample_measurement_test_pb2 from tests.assets.sample_measurement_test_pb2 import ProtobufColor @@ -207,6 +208,7 @@ def test___measurement_service_v2___measure_with_large_array___returns_output( @pytest.fixture(scope="module") def measurement_service() -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" + GlobalTestingState.IsInTestState = True with loopback_measurement.measurement_service.host_service() as service: yield service diff --git a/tests/acceptance/test_streaming_data_measurement.py b/tests/acceptance/test_streaming_data_measurement.py index e3b4fb7ed..96ecd954d 100644 --- a/tests/acceptance/test_streaming_data_measurement.py +++ b/tests/acceptance/test_streaming_data_measurement.py @@ -9,6 +9,7 @@ measurement_service_pb2 as v2_measurement_service_pb2, measurement_service_pb2_grpc as v2_measurement_service_pb2_grpc, ) +from ni_measurementlink_service._internal.utilities.globaltestingstate import GlobalTestingState from ni_measurementlink_service.measurement.service import MeasurementService from tests.assets import sample_streaming_measurement_test_pb2 from tests.utilities import streaming_data_measurement @@ -139,5 +140,6 @@ def _get_serialized_measurement_outputs( @pytest.fixture(scope="module") def measurement_service() -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" + GlobalTestingState.IsInTestState = True with streaming_data_measurement.measurement_service.host_service() as service: yield service diff --git a/tests/acceptance/test_yield_vs_return.py b/tests/acceptance/test_yield_vs_return.py index 9ee16bf85..9b32e1d84 100644 --- a/tests/acceptance/test_yield_vs_return.py +++ b/tests/acceptance/test_yield_vs_return.py @@ -8,6 +8,7 @@ measurement_service_pb2 as v2_measurement_service_pb2, measurement_service_pb2_grpc as v2_measurement_service_pb2_grpc, ) +from ni_measurementlink_service._internal.utilities.globaltestingstate import GlobalTestingState from ni_measurementlink_service.measurement.service import MeasurementService from tests.assets import ui_progress_updates_test_pb2 from tests.utilities import yield_vs_return_measurement @@ -55,5 +56,6 @@ def _get_serialized_measurement_configuration_parameters( @pytest.fixture(scope="module") def measurement_service() -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" + GlobalTestingState.IsInTestState = True with yield_vs_return_measurement.measurement_service.host_service() as service: yield service From 46327653254e95416fd51e0b7957e4789c136518 Mon Sep 17 00:00:00 2001 From: mbattu Date: Tue, 8 Aug 2023 14:33:16 +0530 Subject: [PATCH 03/47] Fix: Killed Discovery Service which was started by Test environment --- ni_measurementlink_service/_internal/discovery_client.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ni_measurementlink_service/_internal/discovery_client.py b/ni_measurementlink_service/_internal/discovery_client.py index 485a617f1..2167b3f70 100644 --- a/ni_measurementlink_service/_internal/discovery_client.py +++ b/ni_measurementlink_service/_internal/discovery_client.py @@ -16,6 +16,7 @@ discovery_service_pb2, discovery_service_pb2_grpc, ) +from ni_measurementlink_service._internal.utilities.globaltestingstate import GlobalTestingState from ni_measurementlink_service.measurement.info import MeasurementInfo, ServiceInfo if sys.platform == "win32": @@ -181,6 +182,13 @@ def unregister_service(self) -> bool: except Exception: _logger.exception("Error in unregistering with discovery service.") return False + finally: + global _discovery_service_subprocess + # Killing the Discovery Service started by the Test environment. + if GlobalTestingState.IsInTestState and _discovery_service_subprocess is not None: + _discovery_service_subprocess.kill() + _discovery_service_subprocess.communicate() + return True def resolve_service(self, provided_interface: str, service_class: str = "") -> ServiceLocation: From 0bc7ea24bbbdbcc94c0024fe14dfc54533960ef1 Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 9 Aug 2023 22:35:58 +0530 Subject: [PATCH 04/47] Fix: Delete globaltestingstate.py --- .../_internal/utilities/globaltestingstate.py | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 ni_measurementlink_service/_internal/utilities/globaltestingstate.py diff --git a/ni_measurementlink_service/_internal/utilities/globaltestingstate.py b/ni_measurementlink_service/_internal/utilities/globaltestingstate.py deleted file mode 100644 index 17e6d2610..000000000 --- a/ni_measurementlink_service/_internal/utilities/globaltestingstate.py +++ /dev/null @@ -1,8 +0,0 @@ -"""Contains indicators to identify the Production/Test environment. -""" - - -class GlobalTestingState: - """Indicator for measurement running in testing environment.""" - - IsInTestState: bool = False From 58aa0f88c1c772c7c6b12006047a6feead75605a Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 9 Aug 2023 22:36:32 +0530 Subject: [PATCH 05/47] Feat: Added a generator class discovery service process --- tests/utilities/discovery_service_process.py | 38 ++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 tests/utilities/discovery_service_process.py diff --git a/tests/utilities/discovery_service_process.py b/tests/utilities/discovery_service_process.py new file mode 100644 index 000000000..43b03e166 --- /dev/null +++ b/tests/utilities/discovery_service_process.py @@ -0,0 +1,38 @@ +"""Generator class to create and terminate Discovery Service instance.""" + +from ni_measurementlink_service._internal.discovery_client import ( + _get_discovery_service_location, + _get_key_file_path, + _service_already_running, + _start_service, +) + + +class DiscoveryServiceProcess: + """Maintains the processes involved in creating and terminating discovery service.""" + + def __init__(self): + """Creates a DiscoveryServiceProcess instance.""" + try: + self._proc = None + key_file_path = _get_key_file_path() + if _service_already_running(key_file_path): + return + self._proc = _start_service(_get_discovery_service_location(), key_file_path) + except Exception: + self._close_discovery_service() + raise + + def __enter__(self): + """Returns the DiscoveryServiceProcess instance.""" + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + """Closes the DiscoveryServiceProcess instance.""" + self._close_discovery_service() + + def _close_discovery_service(self): + if self._proc is not None: + self._proc.kill() + # Use communicate() to close the stdout pipe and wait for the server process to exit. + _, _ = self._proc.communicate() From 7212665a33ccad304b6fd0bb63d91f0e26ca1fd7 Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 9 Aug 2023 22:37:15 +0530 Subject: [PATCH 06/47] Feat: Added generator fixture for yeilding discovery service process --- tests/conftest.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 2fa462b87..48445e29d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,6 +12,7 @@ measurement_service_pb2_grpc as v2_measurement_service_pb2_grpc, ) from ni_measurementlink_service.measurement.service import MeasurementService +from tests.utilities.discovery_service_process import DiscoveryServiceProcess @pytest.fixture(scope="module") @@ -42,3 +43,10 @@ def stub_v1(grpc_channel: grpc.Channel) -> v1_measurement_service_pb2_grpc.Measu def stub_v2(grpc_channel: grpc.Channel) -> v2_measurement_service_pb2_grpc.MeasurementServiceStub: """Test fixture that creates a MeasurementService v2 stub.""" return v2_measurement_service_pb2_grpc.MeasurementServiceStub(grpc_channel) + + +@pytest.fixture(scope="session") +def discovery_service_process() -> Generator[DiscoveryServiceProcess, None, None]: + """Test fixture that creates discovery service process.""" + with DiscoveryServiceProcess() as proc: + yield proc From 73c194d8dbccbf4ca4f7efc370eae5447ffd3fe0 Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 9 Aug 2023 22:37:58 +0530 Subject: [PATCH 07/47] Fix: Returned _discovery_service_subprocess --- .../_internal/discovery_client.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/ni_measurementlink_service/_internal/discovery_client.py b/ni_measurementlink_service/_internal/discovery_client.py index 2167b3f70..f6653530c 100644 --- a/ni_measurementlink_service/_internal/discovery_client.py +++ b/ni_measurementlink_service/_internal/discovery_client.py @@ -16,7 +16,6 @@ discovery_service_pb2, discovery_service_pb2_grpc, ) -from ni_measurementlink_service._internal.utilities.globaltestingstate import GlobalTestingState from ni_measurementlink_service.measurement.info import MeasurementInfo, ServiceInfo if sys.platform == "win32": @@ -182,13 +181,6 @@ def unregister_service(self) -> bool: except Exception: _logger.exception("Error in unregistering with discovery service.") return False - finally: - global _discovery_service_subprocess - # Killing the Discovery Service started by the Test environment. - if GlobalTestingState.IsInTestState and _discovery_service_subprocess is not None: - _discovery_service_subprocess.kill() - _discovery_service_subprocess.communicate() - return True def resolve_service(self, provided_interface: str, service_class: str = "") -> ServiceLocation: @@ -238,7 +230,8 @@ def _ensure_discovery_service_started(key_file_path: pathlib.Path) -> None: return exe_file_path = _get_discovery_service_location() - _start_service(exe_file_path, key_file_path) + global _discovery_service_subprocess # save Popen object to avoid ResourceWarning + _discovery_service_subprocess = _start_service(exe_file_path, key_file_path) def _get_discovery_service_location() -> pathlib.PurePath: @@ -265,9 +258,10 @@ def _key_file_exists(key_file_path: pathlib.Path) -> bool: return key_file_path.is_file() and key_file_path.stat().st_size > 0 -def _start_service(exe_file_path: pathlib.PurePath, key_file_path: pathlib.Path) -> None: +def _start_service( + exe_file_path: pathlib.PurePath, key_file_path: pathlib.Path +) -> subprocess.Popen: """Starts the service at the specified path and wait for the service to get up and running.""" - global _discovery_service_subprocess # save Popen object to avoid ResourceWarning _discovery_service_subprocess = subprocess.Popen( [exe_file_path], cwd=exe_file_path.parent, @@ -280,7 +274,7 @@ def _start_service(exe_file_path: pathlib.PurePath, key_file_path: pathlib.Path) while True: try: with _open_key_file(str(key_file_path)) as _: - return + return _discovery_service_subprocess except IOError: pass if time.time() >= timeout_time: From 7dbb9322b450aa322b00cc1571701d3d1eafdf6a Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 9 Aug 2023 22:38:25 +0530 Subject: [PATCH 08/47] Fix: Updated tests to use generator fixture --- tests/acceptance/test_measurement_service.py | 5 ++--- tests/acceptance/test_streaming_data_measurement.py | 5 ++--- tests/acceptance/test_yield_vs_return.py | 5 ++--- tests/unit/test_discovery_client.py | 2 +- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/tests/acceptance/test_measurement_service.py b/tests/acceptance/test_measurement_service.py index 7c9b757ea..00b241674 100644 --- a/tests/acceptance/test_measurement_service.py +++ b/tests/acceptance/test_measurement_service.py @@ -17,7 +17,6 @@ measurement_service_pb2 as v2_measurement_service_pb2, measurement_service_pb2_grpc as v2_measurement_service_pb2_grpc, ) -from ni_measurementlink_service._internal.utilities.globaltestingstate import GlobalTestingState from ni_measurementlink_service.measurement.service import MeasurementService from tests.assets import sample_measurement_test_pb2 from tests.assets.sample_measurement_test_pb2 import ProtobufColor @@ -206,9 +205,9 @@ def test___measurement_service_v2___measure_with_large_array___returns_output( @pytest.fixture(scope="module") -def measurement_service() -> Generator[MeasurementService, None, None]: +def measurement_service(discovery_service_process) -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" - GlobalTestingState.IsInTestState = True + _ = discovery_service_process with loopback_measurement.measurement_service.host_service() as service: yield service diff --git a/tests/acceptance/test_streaming_data_measurement.py b/tests/acceptance/test_streaming_data_measurement.py index 96ecd954d..9dd0d191d 100644 --- a/tests/acceptance/test_streaming_data_measurement.py +++ b/tests/acceptance/test_streaming_data_measurement.py @@ -9,7 +9,6 @@ measurement_service_pb2 as v2_measurement_service_pb2, measurement_service_pb2_grpc as v2_measurement_service_pb2_grpc, ) -from ni_measurementlink_service._internal.utilities.globaltestingstate import GlobalTestingState from ni_measurementlink_service.measurement.service import MeasurementService from tests.assets import sample_streaming_measurement_test_pb2 from tests.utilities import streaming_data_measurement @@ -138,8 +137,8 @@ def _get_serialized_measurement_outputs( @pytest.fixture(scope="module") -def measurement_service() -> Generator[MeasurementService, None, None]: +def measurement_service(discovery_service_process) -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" - GlobalTestingState.IsInTestState = True + _ = discovery_service_process with streaming_data_measurement.measurement_service.host_service() as service: yield service diff --git a/tests/acceptance/test_yield_vs_return.py b/tests/acceptance/test_yield_vs_return.py index 9b32e1d84..a259fa7af 100644 --- a/tests/acceptance/test_yield_vs_return.py +++ b/tests/acceptance/test_yield_vs_return.py @@ -8,7 +8,6 @@ measurement_service_pb2 as v2_measurement_service_pb2, measurement_service_pb2_grpc as v2_measurement_service_pb2_grpc, ) -from ni_measurementlink_service._internal.utilities.globaltestingstate import GlobalTestingState from ni_measurementlink_service.measurement.service import MeasurementService from tests.assets import ui_progress_updates_test_pb2 from tests.utilities import yield_vs_return_measurement @@ -54,8 +53,8 @@ def _get_serialized_measurement_configuration_parameters( @pytest.fixture(scope="module") -def measurement_service() -> Generator[MeasurementService, None, None]: +def measurement_service(discovery_service_process) -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" - GlobalTestingState.IsInTestState = True + _ = discovery_service_process with yield_vs_return_measurement.measurement_service.host_service() as service: yield service diff --git a/tests/unit/test_discovery_client.py b/tests/unit/test_discovery_client.py index 1cf315c69..33c44ea65 100644 --- a/tests/unit/test_discovery_client.py +++ b/tests/unit/test_discovery_client.py @@ -150,7 +150,7 @@ def test___start_discovery_service___key_file_exist_after_poll___service_start_s ) mock_popen = mocker.patch("subprocess.Popen") - _start_service(exe_file_path, temp_discovery_key_file_path) + _ = _start_service(exe_file_path, temp_discovery_key_file_path) mock_popen.assert_called_once_with( [exe_file_path], From fb468f226e479b94a12f9706d6fa39444961360f Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 10 Aug 2023 00:55:49 +0530 Subject: [PATCH 09/47] Fix: Update reurn types in Context manager discovery_service_process.py --- tests/utilities/discovery_service_process.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tests/utilities/discovery_service_process.py b/tests/utilities/discovery_service_process.py index 43b03e166..00c8ab2c4 100644 --- a/tests/utilities/discovery_service_process.py +++ b/tests/utilities/discovery_service_process.py @@ -1,4 +1,7 @@ -"""Generator class to create and terminate Discovery Service instance.""" +"""Class to create and terminate Discovery Service instance.""" + +from types import TracebackType +from typing import Optional, Type, TypeVar from ni_measurementlink_service._internal.discovery_client import ( _get_discovery_service_location, @@ -7,11 +10,13 @@ _start_service, ) +Self = TypeVar("Self", bound=object) + class DiscoveryServiceProcess: """Maintains the processes involved in creating and terminating discovery service.""" - def __init__(self): + def __init__(self) -> None: """Creates a DiscoveryServiceProcess instance.""" try: self._proc = None @@ -23,15 +28,20 @@ def __init__(self): self._close_discovery_service() raise - def __enter__(self): + def __enter__(self: Self) -> Self: """Returns the DiscoveryServiceProcess instance.""" return self - def __exit__(self, exc_type, exc_val, exc_tb): + def __exit__( + self, + exc_type: Optional[Type[BaseException]], + exc_val: Optional[BaseException], + exc_tb: Optional[TracebackType], + ) -> None: """Closes the DiscoveryServiceProcess instance.""" self._close_discovery_service() - def _close_discovery_service(self): + def _close_discovery_service(self) -> None: if self._proc is not None: self._proc.kill() # Use communicate() to close the stdout pipe and wait for the server process to exit. From 3225c0e56d2bde16e28dd3e343d69e90bbece885 Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 10 Aug 2023 01:22:28 +0530 Subject: [PATCH 10/47] Fix: Import Self from typing_extensions --- tests/utilities/discovery_service_process.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/utilities/discovery_service_process.py b/tests/utilities/discovery_service_process.py index 00c8ab2c4..3b5a371d8 100644 --- a/tests/utilities/discovery_service_process.py +++ b/tests/utilities/discovery_service_process.py @@ -1,7 +1,9 @@ """Class to create and terminate Discovery Service instance.""" from types import TracebackType -from typing import Optional, Type, TypeVar +from typing import Optional, Type + +from typing_extensions import Self from ni_measurementlink_service._internal.discovery_client import ( _get_discovery_service_location, @@ -10,8 +12,6 @@ _start_service, ) -Self = TypeVar("Self", bound=object) - class DiscoveryServiceProcess: """Maintains the processes involved in creating and terminating discovery service.""" From 9c7dd0682bf98f929d00534b295bc7bc4df2ad26 Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 10 Aug 2023 10:16:30 +0530 Subject: [PATCH 11/47] Fix: Removed throw away variable assignments --- tests/acceptance/test_measurement_service.py | 2 +- tests/acceptance/test_streaming_data_measurement.py | 2 +- tests/acceptance/test_yield_vs_return.py | 2 +- tests/unit/test_discovery_client.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/acceptance/test_measurement_service.py b/tests/acceptance/test_measurement_service.py index 00b241674..91e7ff049 100644 --- a/tests/acceptance/test_measurement_service.py +++ b/tests/acceptance/test_measurement_service.py @@ -207,7 +207,7 @@ def test___measurement_service_v2___measure_with_large_array___returns_output( @pytest.fixture(scope="module") def measurement_service(discovery_service_process) -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" - _ = discovery_service_process + discovery_service_process with loopback_measurement.measurement_service.host_service() as service: yield service diff --git a/tests/acceptance/test_streaming_data_measurement.py b/tests/acceptance/test_streaming_data_measurement.py index 9dd0d191d..927701173 100644 --- a/tests/acceptance/test_streaming_data_measurement.py +++ b/tests/acceptance/test_streaming_data_measurement.py @@ -139,6 +139,6 @@ def _get_serialized_measurement_outputs( @pytest.fixture(scope="module") def measurement_service(discovery_service_process) -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" - _ = discovery_service_process + discovery_service_process with streaming_data_measurement.measurement_service.host_service() as service: yield service diff --git a/tests/acceptance/test_yield_vs_return.py b/tests/acceptance/test_yield_vs_return.py index a259fa7af..e112d9505 100644 --- a/tests/acceptance/test_yield_vs_return.py +++ b/tests/acceptance/test_yield_vs_return.py @@ -55,6 +55,6 @@ def _get_serialized_measurement_configuration_parameters( @pytest.fixture(scope="module") def measurement_service(discovery_service_process) -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" - _ = discovery_service_process + discovery_service_process with yield_vs_return_measurement.measurement_service.host_service() as service: yield service diff --git a/tests/unit/test_discovery_client.py b/tests/unit/test_discovery_client.py index b80b4e369..31f33c4c0 100644 --- a/tests/unit/test_discovery_client.py +++ b/tests/unit/test_discovery_client.py @@ -156,7 +156,7 @@ def test___start_discovery_service___key_file_exist_after_poll___service_start_s ) mock_popen = mocker.patch("subprocess.Popen") - _ = _start_service(exe_file_path, temp_discovery_key_file_path) + _start_service(exe_file_path, temp_discovery_key_file_path) mock_popen.assert_called_once_with( [exe_file_path], From d52160d4cc3732b2737c7a64fc155e6100860422 Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 10 Aug 2023 16:30:59 +0530 Subject: [PATCH 12/47] Test: Remove the context manager call --- tests/acceptance/test_measurement_service.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/acceptance/test_measurement_service.py b/tests/acceptance/test_measurement_service.py index 91e7ff049..2d9853cc6 100644 --- a/tests/acceptance/test_measurement_service.py +++ b/tests/acceptance/test_measurement_service.py @@ -205,9 +205,8 @@ def test___measurement_service_v2___measure_with_large_array___returns_output( @pytest.fixture(scope="module") -def measurement_service(discovery_service_process) -> Generator[MeasurementService, None, None]: +def measurement_service() -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" - discovery_service_process with loopback_measurement.measurement_service.host_service() as service: yield service From 044fd2f734a66a01bae71535879b7e8b545f0e52 Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 10 Aug 2023 16:41:36 +0530 Subject: [PATCH 13/47] Test: Don't call discovery_service_process --- tests/acceptance/test_measurement_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/acceptance/test_measurement_service.py b/tests/acceptance/test_measurement_service.py index 2d9853cc6..ca111098b 100644 --- a/tests/acceptance/test_measurement_service.py +++ b/tests/acceptance/test_measurement_service.py @@ -205,7 +205,7 @@ def test___measurement_service_v2___measure_with_large_array___returns_output( @pytest.fixture(scope="module") -def measurement_service() -> Generator[MeasurementService, None, None]: +def measurement_service(discovery_service_process) -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" with loopback_measurement.measurement_service.host_service() as service: yield service From 3da8595d90c61a8c0b812639ba0e0be7cf8453aa Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 10 Aug 2023 16:52:48 +0530 Subject: [PATCH 14/47] Revert: Reverted back test changes --- tests/acceptance/test_measurement_service.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/acceptance/test_measurement_service.py b/tests/acceptance/test_measurement_service.py index ca111098b..91e7ff049 100644 --- a/tests/acceptance/test_measurement_service.py +++ b/tests/acceptance/test_measurement_service.py @@ -207,6 +207,7 @@ def test___measurement_service_v2___measure_with_large_array___returns_output( @pytest.fixture(scope="module") def measurement_service(discovery_service_process) -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" + discovery_service_process with loopback_measurement.measurement_service.host_service() as service: yield service From 102a0286ab890a1a27906c14225cfed7bc6991d2 Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 10 Aug 2023 18:12:06 +0530 Subject: [PATCH 15/47] Test: Check if stub is None or not --- ni_measurementlink_service/_internal/discovery_client.py | 7 +++---- tests/acceptance/test_measurement_service.py | 3 +-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/ni_measurementlink_service/_internal/discovery_client.py b/ni_measurementlink_service/_internal/discovery_client.py index 329ad39b4..874254f11 100644 --- a/ni_measurementlink_service/_internal/discovery_client.py +++ b/ni_measurementlink_service/_internal/discovery_client.py @@ -80,10 +80,9 @@ def __init__( @property def stub(self) -> discovery_service_pb2_grpc.DiscoveryServiceStub: """Get the gRPC stub used to interact with the discovery service.""" - if self._stub is None: - address = _get_discovery_service_address() - channel = grpc.insecure_channel(address) - self._stub = discovery_service_pb2_grpc.DiscoveryServiceStub(channel) + address = _get_discovery_service_address() + channel = grpc.insecure_channel(address) + self._stub = discovery_service_pb2_grpc.DiscoveryServiceStub(channel) return self._stub def register_measurement_service( diff --git a/tests/acceptance/test_measurement_service.py b/tests/acceptance/test_measurement_service.py index 91e7ff049..2d9853cc6 100644 --- a/tests/acceptance/test_measurement_service.py +++ b/tests/acceptance/test_measurement_service.py @@ -205,9 +205,8 @@ def test___measurement_service_v2___measure_with_large_array___returns_output( @pytest.fixture(scope="module") -def measurement_service(discovery_service_process) -> Generator[MeasurementService, None, None]: +def measurement_service() -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" - discovery_service_process with loopback_measurement.measurement_service.host_service() as service: yield service From 3e81232e14cae60f1d181227be207ec113d352ab Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 10 Aug 2023 18:17:36 +0530 Subject: [PATCH 16/47] Revert: Verified stub is actually None --- ni_measurementlink_service/_internal/discovery_client.py | 7 ++++--- tests/acceptance/test_measurement_service.py | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/ni_measurementlink_service/_internal/discovery_client.py b/ni_measurementlink_service/_internal/discovery_client.py index 874254f11..329ad39b4 100644 --- a/ni_measurementlink_service/_internal/discovery_client.py +++ b/ni_measurementlink_service/_internal/discovery_client.py @@ -80,9 +80,10 @@ def __init__( @property def stub(self) -> discovery_service_pb2_grpc.DiscoveryServiceStub: """Get the gRPC stub used to interact with the discovery service.""" - address = _get_discovery_service_address() - channel = grpc.insecure_channel(address) - self._stub = discovery_service_pb2_grpc.DiscoveryServiceStub(channel) + if self._stub is None: + address = _get_discovery_service_address() + channel = grpc.insecure_channel(address) + self._stub = discovery_service_pb2_grpc.DiscoveryServiceStub(channel) return self._stub def register_measurement_service( diff --git a/tests/acceptance/test_measurement_service.py b/tests/acceptance/test_measurement_service.py index 2d9853cc6..91e7ff049 100644 --- a/tests/acceptance/test_measurement_service.py +++ b/tests/acceptance/test_measurement_service.py @@ -205,8 +205,9 @@ def test___measurement_service_v2___measure_with_large_array___returns_output( @pytest.fixture(scope="module") -def measurement_service() -> Generator[MeasurementService, None, None]: +def measurement_service(discovery_service_process) -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" + discovery_service_process with loopback_measurement.measurement_service.host_service() as service: yield service From 822594a8fcf986e381988b38f6a2c1bd1ac7dd55 Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 10 Aug 2023 19:31:13 +0530 Subject: [PATCH 17/47] Test: Don't raise exception --- tests/utilities/discovery_service_process.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/utilities/discovery_service_process.py b/tests/utilities/discovery_service_process.py index 3b5a371d8..55989d21e 100644 --- a/tests/utilities/discovery_service_process.py +++ b/tests/utilities/discovery_service_process.py @@ -26,7 +26,6 @@ def __init__(self) -> None: self._proc = _start_service(_get_discovery_service_location(), key_file_path) except Exception: self._close_discovery_service() - raise def __enter__(self: Self) -> Self: """Returns the DiscoveryServiceProcess instance.""" From e839281488e84fe5f416ab8590e602bffff929ac Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 10 Aug 2023 22:12:01 +0530 Subject: [PATCH 18/47] Fix: Removed throw away varaible assignments --- tests/utilities/discovery_service_process.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utilities/discovery_service_process.py b/tests/utilities/discovery_service_process.py index 55989d21e..a72040f09 100644 --- a/tests/utilities/discovery_service_process.py +++ b/tests/utilities/discovery_service_process.py @@ -44,4 +44,4 @@ def _close_discovery_service(self) -> None: if self._proc is not None: self._proc.kill() # Use communicate() to close the stdout pipe and wait for the server process to exit. - _, _ = self._proc.communicate() + self._proc.communicate() From 4bf3a7f147ea918485a1fdea85524cea66ebe181 Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 10 Aug 2023 22:21:30 +0530 Subject: [PATCH 19/47] Revert: Raise Exception for Discovery Service --- tests/utilities/discovery_service_process.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/utilities/discovery_service_process.py b/tests/utilities/discovery_service_process.py index a72040f09..0048780e3 100644 --- a/tests/utilities/discovery_service_process.py +++ b/tests/utilities/discovery_service_process.py @@ -26,6 +26,7 @@ def __init__(self) -> None: self._proc = _start_service(_get_discovery_service_location(), key_file_path) except Exception: self._close_discovery_service() + raise def __enter__(self: Self) -> Self: """Returns the DiscoveryServiceProcess instance.""" From 300c526568edd150b311be1e7fee3d3496527129 Mon Sep 17 00:00:00 2001 From: mbattu Date: Mon, 14 Aug 2023 11:11:05 +0530 Subject: [PATCH 20/47] Fix: Skipped tests for non-windows & no measurement registration file --- tests/conftest.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 48445e29d..06935df4f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,10 +1,13 @@ """Pytest configuration file.""" +import os import pathlib +import sys from typing import Generator import grpc import pytest +from ni_measurementlink_service._internal.discovery_client import _get_registration_json_file_path from ni_measurementlink_service._internal.stubs.ni.measurementlink.measurement.v1 import ( measurement_service_pb2_grpc as v1_measurement_service_pb2_grpc, ) @@ -48,5 +51,16 @@ def stub_v2(grpc_channel: grpc.Channel) -> v2_measurement_service_pb2_grpc.Measu @pytest.fixture(scope="session") def discovery_service_process() -> Generator[DiscoveryServiceProcess, None, None]: """Test fixture that creates discovery service process.""" - with DiscoveryServiceProcess() as proc: - yield proc + if sys.platform != "win32": + pytest.skip( + f"Platform {sys.platform} is not supported for starting discovery service." + "Could not proceed to run the tests." + ) + elif os.path.exists(_get_registration_json_file_path()): + pytest.skip( + "MeasurementLink registration file not found or MeasurementLink not installed." + "Could not proceed to run the tests. Install MeasurementLink to create the registration file." + ) + else: + with DiscoveryServiceProcess() as proc: + yield proc From 00631478992e6a5a38ffab731bb22d0531eda163 Mon Sep 17 00:00:00 2001 From: mbattu Date: Mon, 14 Aug 2023 11:20:12 +0530 Subject: [PATCH 21/47] Fix: Correct the if condition --- tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 06935df4f..680d05226 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -56,7 +56,7 @@ def discovery_service_process() -> Generator[DiscoveryServiceProcess, None, None f"Platform {sys.platform} is not supported for starting discovery service." "Could not proceed to run the tests." ) - elif os.path.exists(_get_registration_json_file_path()): + elif not os.path.exists(_get_registration_json_file_path()): pytest.skip( "MeasurementLink registration file not found or MeasurementLink not installed." "Could not proceed to run the tests. Install MeasurementLink to create the registration file." From 6589f60b78b05953cb9c6d7f4173042f8aadf275 Mon Sep 17 00:00:00 2001 From: mbattu Date: Mon, 14 Aug 2023 14:44:39 +0530 Subject: [PATCH 22/47] Fix: Use Pathlib instead of os --- tests/conftest.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 680d05226..8fb22200b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,4 @@ """Pytest configuration file.""" -import os import pathlib import sys from typing import Generator @@ -56,7 +55,7 @@ def discovery_service_process() -> Generator[DiscoveryServiceProcess, None, None f"Platform {sys.platform} is not supported for starting discovery service." "Could not proceed to run the tests." ) - elif not os.path.exists(_get_registration_json_file_path()): + elif not pathlib.Path.exists(_get_registration_json_file_path()): pytest.skip( "MeasurementLink registration file not found or MeasurementLink not installed." "Could not proceed to run the tests. Install MeasurementLink to create the registration file." From ce9c6f782e71c0138ea386b628df7fa882044a39 Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 16 Aug 2023 17:14:18 +0530 Subject: [PATCH 23/47] Feat: Dont start measurement on unsuccessful register with discovery service --- .../_internal/service_manager.py | 10 ++++++++-- ni_measurementlink_service/measurement/service.py | 6 ++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/ni_measurementlink_service/_internal/service_manager.py b/ni_measurementlink_service/_internal/service_manager.py index 5700bc47d..f5f471146 100644 --- a/ni_measurementlink_service/_internal/service_manager.py +++ b/ni_measurementlink_service/_internal/service_manager.py @@ -101,10 +101,16 @@ def start( port = str(self.server.add_insecure_port("[::]:0")) self.server.start() _logger.info("Measurement service hosted on port: %s", port) - self.discovery_client.register_measurement_service(port, service_info, measurement_info) + is_measurement_registered = self.discovery_client.register_measurement_service( + port, service_info, measurement_info + ) self.port = port - return port + + # Return port number if measurement is successfully registered with the Discovery Service. + if is_measurement_registered: + return port + return None def stop(self) -> None: """Close the Service after un-registering with discovery service and cleanups.""" diff --git a/ni_measurementlink_service/measurement/service.py b/ni_measurementlink_service/measurement/service.py index 139cee3a3..2485c13be 100644 --- a/ni_measurementlink_service/measurement/service.py +++ b/ni_measurementlink_service/measurement/service.py @@ -375,14 +375,16 @@ def host_service(self) -> MeasurementService: """ if self.measure_function is None: raise Exception("Error, must register measurement method.") - self.grpc_service.start( + is_service_started = self.grpc_service.start( self.measurement_info, self.service_info, self.configuration_parameter_list, self.output_parameter_list, self.measure_function, ) - return self + if is_service_started: + return self + raise Exception("Error in starting the measurement service.") def _make_annotations_dict( self, From 4f2494779fdd8fa0c6d87608733776d754ab0fbf Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 16 Aug 2023 17:14:49 +0530 Subject: [PATCH 24/47] Feat: Added tests to cover the exception cases --- tests/integration/test_service_manager.py | 42 +++++++++++++++++++++-- tests/unit/test_service.py | 15 ++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_service_manager.py b/tests/integration/test_service_manager.py index ef1d105f1..dbbb76fc5 100644 --- a/tests/integration/test_service_manager.py +++ b/tests/integration/test_service_manager.py @@ -14,7 +14,10 @@ measurement_service_pb2_grpc, ) from tests.utilities import loopback_measurement -from tests.utilities.fake_discovery_service import FakeDiscoveryServiceStub +from tests.utilities.fake_discovery_service import ( + FakeDiscoveryServiceStub, + FakeDiscoveryServiceStubError, +) def test___grpc_service___start_service___service_hosted(grpc_service: GrpcService): @@ -43,6 +46,24 @@ def test___grpc_service_without_discovery_service___start_service___service_host _validate_if_service_running_by_making_rpc(port_number) +@pytest.mark.expect_discovery_service_stub_error(True) +def test___grpc_service___start_service_error_registering_measurement___raises_type_error( + grpc_service: GrpcService, +): + port_number = grpc_service.start( + loopback_measurement.measurement_service.measurement_info, + loopback_measurement.measurement_service.service_info, + loopback_measurement.measurement_service.configuration_parameter_list, + loopback_measurement.measurement_service.output_parameter_list, + loopback_measurement.measurement_service.measure_function, + ) + + with pytest.raises(Exception): + _validate_if_service_running_by_making_rpc(port_number) + + assert not port_number + + def test___grpc_service_started___stop_service___service_stopped(grpc_service: GrpcService): port_number = grpc_service.start( loopback_measurement.measurement_service.measurement_info, @@ -71,8 +92,13 @@ def discovery_client(discovery_service_stub: FakeDiscoveryServiceStub) -> Discov @pytest.fixture -def discovery_service_stub() -> FakeDiscoveryServiceStub: - """Create a FakeDiscoveryServiceStub.""" +def discovery_service_stub(request) -> FakeDiscoveryServiceStub: + """Create a valid or error FakeDiscoveryServiceStub.""" + expect_discovery_service_stub_error = _get_marker_value( + request, "expect_discovery_service_stub_error" + ) + if expect_discovery_service_stub_error: + return FakeDiscoveryServiceStubError() return FakeDiscoveryServiceStub() @@ -84,3 +110,13 @@ def _validate_if_service_running_by_making_rpc(port_number): with grpc.insecure_channel("localhost:" + port_number) as channel: stub = measurement_service_pb2_grpc.MeasurementServiceStub(channel) stub.GetMetadata(measurement_service_pb2.GetMetadataRequest()) # RPC call + + +def _get_marker_value(request, marker_name, default=None): + """Gets the value of a pytest marker based on the marker name.""" + marker_value = default + for marker in request.node.iter_markers(): + if marker.name == marker_name: # only look at markers with valid argument name + marker_value = marker.args[0] # assume single parameter in marker + + return marker_value diff --git a/tests/unit/test_service.py b/tests/unit/test_service.py index 6335786dd..4f79556b7 100644 --- a/tests/unit/test_service.py +++ b/tests/unit/test_service.py @@ -5,6 +5,7 @@ from typing import List, Type import pytest +from pytest_mock import MockerFixture from ni_measurementlink_service.measurement.info import DataType, TypeSpecialization from ni_measurementlink_service.measurement.service import MeasurementService @@ -350,6 +351,20 @@ def test___measurement_service___add_configuration_with_invalid_enum_type___rais ) +def test___measurement_service___host_service_with_grpc_service_not_started___raises_type_error( + measurement_service: MeasurementService, + mocker: MockerFixture, +): + measurement_service.register_measurement(_fake_measurement_function) + mocker.patch( + "ni_measurementlink_service._internal.service_manager.GrpcService.start", + return_value=None, + ) + + with pytest.raises(Exception): + measurement_service.host_service() + + @pytest.fixture def measurement_service(test_assets_directory: pathlib.Path) -> MeasurementService: """Create a MeasurementService.""" From 58415017ae48bf4baddc7cc23bcd2133537a0858 Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 16 Aug 2023 17:16:50 +0530 Subject: [PATCH 25/47] Feat: Added expect_discovery_service_stub_error marker in pyproject.toml --- pyproject.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index c08b8fd4f..fb2f5a16c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -70,6 +70,10 @@ build-backend = "poetry.core.masonry.api" addopts = "--doctest-modules --strict-markers" filterwarnings = ["always::ImportWarning", "always::ResourceWarning"] testpaths = ["tests"] +markers = [ + # Defines custom markers used by measurement link python tests. Prevents PytestUnknownMarkWarning. + "expect_discovery_service_stub_error: boolean specifying that the fake discovery service stub is valid or errored." +] [tool.mypy] warn_unused_configs = true From 6953af666b35ad48f9ade73130538582d0a0474e Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 16 Aug 2023 17:27:59 +0530 Subject: [PATCH 26/47] Fix: Merge errors --- tests/acceptance/test_measurement_service.py | 1 - tests/acceptance/test_streaming_data_measurement.py | 1 - tests/acceptance/test_yield_vs_return.py | 1 - 3 files changed, 3 deletions(-) diff --git a/tests/acceptance/test_measurement_service.py b/tests/acceptance/test_measurement_service.py index 91e7ff049..ca111098b 100644 --- a/tests/acceptance/test_measurement_service.py +++ b/tests/acceptance/test_measurement_service.py @@ -207,7 +207,6 @@ def test___measurement_service_v2___measure_with_large_array___returns_output( @pytest.fixture(scope="module") def measurement_service(discovery_service_process) -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" - discovery_service_process with loopback_measurement.measurement_service.host_service() as service: yield service diff --git a/tests/acceptance/test_streaming_data_measurement.py b/tests/acceptance/test_streaming_data_measurement.py index 927701173..2b2aab4a2 100644 --- a/tests/acceptance/test_streaming_data_measurement.py +++ b/tests/acceptance/test_streaming_data_measurement.py @@ -139,6 +139,5 @@ def _get_serialized_measurement_outputs( @pytest.fixture(scope="module") def measurement_service(discovery_service_process) -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" - discovery_service_process with streaming_data_measurement.measurement_service.host_service() as service: yield service diff --git a/tests/acceptance/test_yield_vs_return.py b/tests/acceptance/test_yield_vs_return.py index e112d9505..64cdfbf4d 100644 --- a/tests/acceptance/test_yield_vs_return.py +++ b/tests/acceptance/test_yield_vs_return.py @@ -55,6 +55,5 @@ def _get_serialized_measurement_configuration_parameters( @pytest.fixture(scope="module") def measurement_service(discovery_service_process) -> Generator[MeasurementService, None, None]: """Test fixture that creates and hosts a measurement service.""" - discovery_service_process with yield_vs_return_measurement.measurement_service.host_service() as service: yield service From f06592cd7f604ce50005f546b73947be5f4c5ef4 Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 16 Aug 2023 17:31:35 +0530 Subject: [PATCH 27/47] Fix: Merge errors --- ni_measurementlink_service/_internal/discovery_client.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ni_measurementlink_service/_internal/discovery_client.py b/ni_measurementlink_service/_internal/discovery_client.py index 41eff4d20..281e8092f 100644 --- a/ni_measurementlink_service/_internal/discovery_client.py +++ b/ni_measurementlink_service/_internal/discovery_client.py @@ -258,7 +258,6 @@ def _key_file_exists(key_file_path: pathlib.Path) -> bool: return key_file_path.is_file() and key_file_path.stat().st_size > 0 - def _start_service( exe_file_path: pathlib.PurePath, key_file_path: pathlib.Path ) -> subprocess.Popen: From a8c27bab66de5325a52981e753a4a73c5183b9fd Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 16 Aug 2023 18:06:08 +0530 Subject: [PATCH 28/47] Test: Fix Mypy error: return nothing instead of None --- ni_measurementlink_service/_internal/service_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ni_measurementlink_service/_internal/service_manager.py b/ni_measurementlink_service/_internal/service_manager.py index f5f471146..a3511edc9 100644 --- a/ni_measurementlink_service/_internal/service_manager.py +++ b/ni_measurementlink_service/_internal/service_manager.py @@ -110,7 +110,7 @@ def start( # Return port number if measurement is successfully registered with the Discovery Service. if is_measurement_registered: return port - return None + return def stop(self) -> None: """Close the Service after un-registering with discovery service and cleanups.""" From bf8e8d4b961c83bfc8fb83d4f41b27273aa56af0 Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 16 Aug 2023 18:09:08 +0530 Subject: [PATCH 29/47] Test: Mypy error: Don't specify return type --- ni_measurementlink_service/_internal/service_manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ni_measurementlink_service/_internal/service_manager.py b/ni_measurementlink_service/_internal/service_manager.py index a3511edc9..5d06985b3 100644 --- a/ni_measurementlink_service/_internal/service_manager.py +++ b/ni_measurementlink_service/_internal/service_manager.py @@ -53,7 +53,7 @@ def start( configuration_parameter_list: List[ParameterMetadata], output_parameter_list: List[ParameterMetadata], measure_function: Callable, - ) -> str: + ): """Host a gRPC service with the registered measurement method. Args: @@ -110,7 +110,7 @@ def start( # Return port number if measurement is successfully registered with the Discovery Service. if is_measurement_registered: return port - return + return None def stop(self) -> None: """Close the Service after un-registering with discovery service and cleanups.""" From 31b0ab211e7a2292b7c54b695d0b97d2e8827069 Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 16 Aug 2023 18:25:35 +0530 Subject: [PATCH 30/47] Fix: Updated test case name --- tests/unit/test_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_service.py b/tests/unit/test_service.py index 4f79556b7..740a912ad 100644 --- a/tests/unit/test_service.py +++ b/tests/unit/test_service.py @@ -351,7 +351,7 @@ def test___measurement_service___add_configuration_with_invalid_enum_type___rais ) -def test___measurement_service___host_service_with_grpc_service_not_started___raises_type_error( +def test___measurement_service___host_service_with_grpc_service_not_started___raises_error( measurement_service: MeasurementService, mocker: MockerFixture, ): From f6e4989e5ce03a39070c46dcf6c4af1375a8cf01 Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 16 Aug 2023 18:26:30 +0530 Subject: [PATCH 31/47] Fix: Update test case name --- tests/integration/test_service_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_service_manager.py b/tests/integration/test_service_manager.py index dbbb76fc5..6b90c5bbe 100644 --- a/tests/integration/test_service_manager.py +++ b/tests/integration/test_service_manager.py @@ -47,7 +47,7 @@ def test___grpc_service_without_discovery_service___start_service___service_host @pytest.mark.expect_discovery_service_stub_error(True) -def test___grpc_service___start_service_error_registering_measurement___raises_type_error( +def test___grpc_service___start_service_error_registering_measurement___raises_error( grpc_service: GrpcService, ): port_number = grpc_service.start( From 99f258c130433e6f315ce968fe837064345e8c37 Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 16 Aug 2023 22:58:03 +0530 Subject: [PATCH 32/47] Refractor: if else in start method in service_manager.py --- ni_measurementlink_service/_internal/service_manager.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ni_measurementlink_service/_internal/service_manager.py b/ni_measurementlink_service/_internal/service_manager.py index 5d06985b3..e3b5b43b1 100644 --- a/ni_measurementlink_service/_internal/service_manager.py +++ b/ni_measurementlink_service/_internal/service_manager.py @@ -108,9 +108,7 @@ def start( self.port = port # Return port number if measurement is successfully registered with the Discovery Service. - if is_measurement_registered: - return port - return None + return port if is_measurement_registered else None def stop(self) -> None: """Close the Service after un-registering with discovery service and cleanups.""" From c9189d62c7988090ed2c03a5357fa359034d8383 Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 16 Aug 2023 23:18:41 +0530 Subject: [PATCH 33/47] Refractor: Used Union to specify different return types --- ni_measurementlink_service/_internal/service_manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ni_measurementlink_service/_internal/service_manager.py b/ni_measurementlink_service/_internal/service_manager.py index e3b5b43b1..c0a256cb0 100644 --- a/ni_measurementlink_service/_internal/service_manager.py +++ b/ni_measurementlink_service/_internal/service_manager.py @@ -1,5 +1,5 @@ import logging -from typing import Callable, List, Optional +from typing import Callable, List, Optional, Union import grpc from grpc.framework.foundation import logging_pool @@ -53,7 +53,7 @@ def start( configuration_parameter_list: List[ParameterMetadata], output_parameter_list: List[ParameterMetadata], measure_function: Callable, - ): + ) -> Union[str, None]: """Host a gRPC service with the registered measurement method. Args: From bae16495902ac2ebc335634ed5c31e808bd116c5 Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 16 Aug 2023 23:40:16 +0530 Subject: [PATCH 34/47] Refractor: Naming changes, if else --- ni_measurementlink_service/measurement/service.py | 4 ++-- tests/integration/test_service_manager.py | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/ni_measurementlink_service/measurement/service.py b/ni_measurementlink_service/measurement/service.py index 2485c13be..d67f9f01c 100644 --- a/ni_measurementlink_service/measurement/service.py +++ b/ni_measurementlink_service/measurement/service.py @@ -375,14 +375,14 @@ def host_service(self) -> MeasurementService: """ if self.measure_function is None: raise Exception("Error, must register measurement method.") - is_service_started = self.grpc_service.start( + port = self.grpc_service.start( self.measurement_info, self.service_info, self.configuration_parameter_list, self.output_parameter_list, self.measure_function, ) - if is_service_started: + if port: return self raise Exception("Error in starting the measurement service.") diff --git a/tests/integration/test_service_manager.py b/tests/integration/test_service_manager.py index 6b90c5bbe..6a5f8a42f 100644 --- a/tests/integration/test_service_manager.py +++ b/tests/integration/test_service_manager.py @@ -97,9 +97,7 @@ def discovery_service_stub(request) -> FakeDiscoveryServiceStub: expect_discovery_service_stub_error = _get_marker_value( request, "expect_discovery_service_stub_error" ) - if expect_discovery_service_stub_error: - return FakeDiscoveryServiceStubError() - return FakeDiscoveryServiceStub() + return FakeDiscoveryServiceStubError() if expect_discovery_service_stub_error else FakeDiscoveryServiceStub() def _validate_if_service_running_by_making_rpc(port_number): From e15eec9adec9c316b858a6fa86ccefb75f47c09a Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 16 Aug 2023 23:41:44 +0530 Subject: [PATCH 35/47] Refractor: assert condition --- tests/integration/test_service_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_service_manager.py b/tests/integration/test_service_manager.py index 6a5f8a42f..2fac61ff9 100644 --- a/tests/integration/test_service_manager.py +++ b/tests/integration/test_service_manager.py @@ -61,7 +61,7 @@ def test___grpc_service___start_service_error_registering_measurement___raises_e with pytest.raises(Exception): _validate_if_service_running_by_making_rpc(port_number) - assert not port_number + assert port_number is None def test___grpc_service_started___stop_service___service_stopped(grpc_service: GrpcService): From ecffed0cbbbb50fca99013a28c08262944938027 Mon Sep 17 00:00:00 2001 From: mbattu Date: Wed, 16 Aug 2023 23:45:58 +0530 Subject: [PATCH 36/47] Fix: Lint errors --- tests/integration/test_service_manager.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_service_manager.py b/tests/integration/test_service_manager.py index 2fac61ff9..1ebee4f82 100644 --- a/tests/integration/test_service_manager.py +++ b/tests/integration/test_service_manager.py @@ -97,7 +97,11 @@ def discovery_service_stub(request) -> FakeDiscoveryServiceStub: expect_discovery_service_stub_error = _get_marker_value( request, "expect_discovery_service_stub_error" ) - return FakeDiscoveryServiceStubError() if expect_discovery_service_stub_error else FakeDiscoveryServiceStub() + return ( + FakeDiscoveryServiceStubError() + if expect_discovery_service_stub_error + else FakeDiscoveryServiceStub() + ) def _validate_if_service_running_by_making_rpc(port_number): From 00a7200e24d0900aab9da41d06cd9529d6599e09 Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 17 Aug 2023 00:19:43 +0530 Subject: [PATCH 37/47] Refractor: Use Optional instead of Union --- ni_measurementlink_service/_internal/service_manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ni_measurementlink_service/_internal/service_manager.py b/ni_measurementlink_service/_internal/service_manager.py index c0a256cb0..6d42b2cba 100644 --- a/ni_measurementlink_service/_internal/service_manager.py +++ b/ni_measurementlink_service/_internal/service_manager.py @@ -1,5 +1,5 @@ import logging -from typing import Callable, List, Optional, Union +from typing import Callable, List, Optional import grpc from grpc.framework.foundation import logging_pool @@ -53,7 +53,7 @@ def start( configuration_parameter_list: List[ParameterMetadata], output_parameter_list: List[ParameterMetadata], measure_function: Callable, - ) -> Union[str, None]: + ) -> Optional[str]: """Host a gRPC service with the registered measurement method. Args: From 2839c8c620465e17e7e3b59dcfc7cc0a5a0bb0bb Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 17 Aug 2023 22:01:07 +0530 Subject: [PATCH 38/47] Fix: Updated exceptions to be raised from discovery_client.py --- .../_internal/discovery_client.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/ni_measurementlink_service/_internal/discovery_client.py b/ni_measurementlink_service/_internal/discovery_client.py index 281e8092f..debb33a89 100644 --- a/ni_measurementlink_service/_internal/discovery_client.py +++ b/ni_measurementlink_service/_internal/discovery_client.py @@ -132,15 +132,15 @@ def register_measurement_service( ) else: _logger.exception("Error in registering with discovery service.") - return False - except FileNotFoundError: + raise grpc.RpcError(type(e).__name__, e.args[0]) from e + except FileNotFoundError as e: _logger.error( "Unable to register with discovery service. Possible reason: discovery service not running." ) - return False - except Exception: + raise FileNotFoundError(e.strerror, e.filename) from e + except Exception as e: _logger.exception("Error in registering with discovery service.") - return False + raise Exception(type(e).__name__, e.args[0]) from e return True def unregister_service(self) -> bool: @@ -172,15 +172,15 @@ def unregister_service(self) -> bool: ) else: _logger.exception("Error in unregistering with discovery service.") - return False - except FileNotFoundError: + raise Exception(type(e).__name__, e.args[0]) from e + except FileNotFoundError as e: _logger.error( "Unable to unregister with discovery service. Possible reason: discovery service not running." ) - return False - except Exception: + raise FileNotFoundError(e.strerror, e.filename) from e + except Exception as e: _logger.exception("Error in unregistering with discovery service.") - return False + raise Exception(type(e).__name__, e.args[0]) from e return True def resolve_service(self, provided_interface: str, service_class: str = "") -> ServiceLocation: From 150599ca3a62252984432c8c873ca1492b79aa65 Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 17 Aug 2023 22:01:34 +0530 Subject: [PATCH 39/47] Fix: Updated caller to catch the expections from called methods --- .../_internal/service_manager.py | 83 ++++++++++--------- .../measurement/service.py | 23 ++--- 2 files changed, 55 insertions(+), 51 deletions(-) diff --git a/ni_measurementlink_service/_internal/service_manager.py b/ni_measurementlink_service/_internal/service_manager.py index 6d42b2cba..d9dda0173 100644 --- a/ni_measurementlink_service/_internal/service_manager.py +++ b/ni_measurementlink_service/_internal/service_manager.py @@ -53,7 +53,7 @@ def start( configuration_parameter_list: List[ParameterMetadata], output_parameter_list: List[ParameterMetadata], measure_function: Callable, - ) -> Optional[str]: + ) -> str: """Host a gRPC service with the registered measurement method. Args: @@ -73,45 +73,48 @@ def start( int: The port number of the server """ - self.server = grpc.server( - logging_pool.pool(max_workers=10), - options=[ - ("grpc.max_receive_message_length", -1), - ("grpc.max_send_message_length", -1), - ], - ) - servicer_v1 = MeasurementServiceServicerV1( - measurement_info, - configuration_parameter_list, - output_parameter_list, - measure_function, - ) - v1_measurement_service_pb2_grpc.add_MeasurementServiceServicer_to_server( - servicer_v1, self.server - ) - servicer_v2 = MeasurementServiceServicerV2( - measurement_info, - configuration_parameter_list, - output_parameter_list, - measure_function, - ) - v2_measurement_service_pb2_grpc.add_MeasurementServiceServicer_to_server( - servicer_v2, self.server - ) - port = str(self.server.add_insecure_port("[::]:0")) - self.server.start() - _logger.info("Measurement service hosted on port: %s", port) - is_measurement_registered = self.discovery_client.register_measurement_service( - port, service_info, measurement_info - ) - - self.port = port - - # Return port number if measurement is successfully registered with the Discovery Service. - return port if is_measurement_registered else None + try: + self.server = grpc.server( + logging_pool.pool(max_workers=10), + options=[ + ("grpc.max_receive_message_length", -1), + ("grpc.max_send_message_length", -1), + ], + ) + servicer_v1 = MeasurementServiceServicerV1( + measurement_info, + configuration_parameter_list, + output_parameter_list, + measure_function, + ) + v1_measurement_service_pb2_grpc.add_MeasurementServiceServicer_to_server( + servicer_v1, self.server + ) + servicer_v2 = MeasurementServiceServicerV2( + measurement_info, + configuration_parameter_list, + output_parameter_list, + measure_function, + ) + v2_measurement_service_pb2_grpc.add_MeasurementServiceServicer_to_server( + servicer_v2, self.server + ) + port = str(self.server.add_insecure_port("[::]:0")) + self.server.start() + _logger.info("Measurement service hosted on port: %s", port) + self.discovery_client.register_measurement_service(port, service_info, measurement_info) + + self.port = port + + return port + except Exception as e: + raise e def stop(self) -> None: """Close the Service after un-registering with discovery service and cleanups.""" - self.discovery_client.unregister_service() - self.server.stop(5) - _logger.info("Measurement service closed.") + try: + self.discovery_client.unregister_service() + self.server.stop(5) + _logger.info("Measurement service closed.") + except Exception as e: + raise Exception(e) diff --git a/ni_measurementlink_service/measurement/service.py b/ni_measurementlink_service/measurement/service.py index d67f9f01c..fbe4887f6 100644 --- a/ni_measurementlink_service/measurement/service.py +++ b/ni_measurementlink_service/measurement/service.py @@ -373,18 +373,19 @@ def host_service(self) -> MeasurementService: Exception: If register measurement methods not available. """ - if self.measure_function is None: - raise Exception("Error, must register measurement method.") - port = self.grpc_service.start( - self.measurement_info, - self.service_info, - self.configuration_parameter_list, - self.output_parameter_list, - self.measure_function, - ) - if port: + try: + if self.measure_function is None: + raise Exception("Error, must register measurement method.") + self.grpc_service.start( + self.measurement_info, + self.service_info, + self.configuration_parameter_list, + self.output_parameter_list, + self.measure_function, + ) return self - raise Exception("Error in starting the measurement service.") + except Exception as e: + raise e def _make_annotations_dict( self, From 3fdb8c614505808accf7cd7be3cc5d7181afdc6c Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 17 Aug 2023 22:01:52 +0530 Subject: [PATCH 40/47] Fix: Test updates on error handling --- tests/integration/test_service_manager.py | 45 +++++++++-------------- tests/unit/test_discovery_client.py | 11 +++--- tests/unit/test_service.py | 2 +- 3 files changed, 23 insertions(+), 35 deletions(-) diff --git a/tests/integration/test_service_manager.py b/tests/integration/test_service_manager.py index 1ebee4f82..d0f196dbf 100644 --- a/tests/integration/test_service_manager.py +++ b/tests/integration/test_service_manager.py @@ -46,22 +46,18 @@ def test___grpc_service_without_discovery_service___start_service___service_host _validate_if_service_running_by_making_rpc(port_number) -@pytest.mark.expect_discovery_service_stub_error(True) +@pytest.mark.parametrize("expect_discovery_service_error_stub", [True]) def test___grpc_service___start_service_error_registering_measurement___raises_error( grpc_service: GrpcService, ): - port_number = grpc_service.start( - loopback_measurement.measurement_service.measurement_info, - loopback_measurement.measurement_service.service_info, - loopback_measurement.measurement_service.configuration_parameter_list, - loopback_measurement.measurement_service.output_parameter_list, - loopback_measurement.measurement_service.measure_function, - ) - with pytest.raises(Exception): - _validate_if_service_running_by_making_rpc(port_number) - - assert port_number is None + grpc_service.start( + loopback_measurement.measurement_service.measurement_info, + loopback_measurement.measurement_service.service_info, + loopback_measurement.measurement_service.configuration_parameter_list, + loopback_measurement.measurement_service.output_parameter_list, + loopback_measurement.measurement_service.measure_function, + ) def test___grpc_service_started___stop_service___service_stopped(grpc_service: GrpcService): @@ -92,18 +88,21 @@ def discovery_client(discovery_service_stub: FakeDiscoveryServiceStub) -> Discov @pytest.fixture -def discovery_service_stub(request) -> FakeDiscoveryServiceStub: - """Create a valid or error FakeDiscoveryServiceStub.""" - expect_discovery_service_stub_error = _get_marker_value( - request, "expect_discovery_service_stub_error" - ) +def discovery_service_stub(expect_discovery_service_error_stub: bool) -> FakeDiscoveryServiceStub: + """Create a valid/error stub based on expect_discovery_service_error_stub value.""" return ( FakeDiscoveryServiceStubError() - if expect_discovery_service_stub_error + if expect_discovery_service_error_stub else FakeDiscoveryServiceStub() ) +@pytest.fixture +def expect_discovery_service_error_stub() -> bool: + """Boolean to choose between FakeDiscoveryServiceStub and FakeDiscoveryServiceStubError.""" + return False + + def _validate_if_service_running_by_making_rpc(port_number): """Implicit validation of running service. @@ -112,13 +111,3 @@ def _validate_if_service_running_by_making_rpc(port_number): with grpc.insecure_channel("localhost:" + port_number) as channel: stub = measurement_service_pb2_grpc.MeasurementServiceStub(channel) stub.GetMetadata(measurement_service_pb2.GetMetadataRequest()) # RPC call - - -def _get_marker_value(request, marker_name, default=None): - """Gets the value of a pytest marker based on the marker name.""" - marker_value = default - for marker in request.node.iter_markers(): - if marker.name == marker_name: # only look at markers with valid argument name - marker_value = marker.args[0] # assume single parameter in marker - - return marker_value diff --git a/tests/unit/test_discovery_client.py b/tests/unit/test_discovery_client.py index 31f33c4c0..f4ec58acd 100644 --- a/tests/unit/test_discovery_client.py +++ b/tests/unit/test_discovery_client.py @@ -167,7 +167,7 @@ def test___start_discovery_service___key_file_exist_after_poll___service_start_s ) -def test___discovery_service_exe_unavailable___register_service___registration_failure( +def test___discovery_service_exe_unavailable___register_service___raises_file_not_found_error( mocker: MockerFixture, temp_discovery_key_file_path: pathlib.Path, temp_registration_json_file_path: pathlib.Path, @@ -181,13 +181,12 @@ def test___discovery_service_exe_unavailable___register_service___registration_f return_value=temp_registration_json_file_path, ) mocker.patch("subprocess.Popen", side_effect=FileNotFoundError) - discovery_client = DiscoveryClient() - registration_success_flag = discovery_client.register_measurement_service( - _TEST_SERVICE_PORT, _TEST_SERVICE_INFO, _TEST_MEASUREMENT_INFO - ) - assert not registration_success_flag + with pytest.raises(FileNotFoundError): + discovery_client.register_measurement_service( + _TEST_SERVICE_PORT, _TEST_SERVICE_INFO, _TEST_MEASUREMENT_INFO + ) @pytest.fixture(scope="module") diff --git a/tests/unit/test_service.py b/tests/unit/test_service.py index 740a912ad..7a14ed6ce 100644 --- a/tests/unit/test_service.py +++ b/tests/unit/test_service.py @@ -358,7 +358,7 @@ def test___measurement_service___host_service_with_grpc_service_not_started___ra measurement_service.register_measurement(_fake_measurement_function) mocker.patch( "ni_measurementlink_service._internal.service_manager.GrpcService.start", - return_value=None, + side_effect=Exception, ) with pytest.raises(Exception): From 854b5a850158f438d4040e66e5b0e763c9904ca0 Mon Sep 17 00:00:00 2001 From: mbattu Date: Thu, 17 Aug 2023 22:04:51 +0530 Subject: [PATCH 41/47] Revert: Removed new marker in pyproject.toml --- pyproject.toml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index fb2f5a16c..c08b8fd4f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -70,10 +70,6 @@ build-backend = "poetry.core.masonry.api" addopts = "--doctest-modules --strict-markers" filterwarnings = ["always::ImportWarning", "always::ResourceWarning"] testpaths = ["tests"] -markers = [ - # Defines custom markers used by measurement link python tests. Prevents PytestUnknownMarkWarning. - "expect_discovery_service_stub_error: boolean specifying that the fake discovery service stub is valid or errored." -] [tool.mypy] warn_unused_configs = true From 328ae7f4bf680b68b90a2d40a00d396b21f4769d Mon Sep 17 00:00:00 2001 From: Mounika Battu Date: Mon, 21 Aug 2023 23:52:14 +0530 Subject: [PATCH 42/47] Fix: Updated raise with no arguments --- .../_internal/discovery_client.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/ni_measurementlink_service/_internal/discovery_client.py b/ni_measurementlink_service/_internal/discovery_client.py index debb33a89..edfaf0138 100644 --- a/ni_measurementlink_service/_internal/discovery_client.py +++ b/ni_measurementlink_service/_internal/discovery_client.py @@ -132,15 +132,15 @@ def register_measurement_service( ) else: _logger.exception("Error in registering with discovery service.") - raise grpc.RpcError(type(e).__name__, e.args[0]) from e - except FileNotFoundError as e: + raise + except FileNotFoundError: _logger.error( "Unable to register with discovery service. Possible reason: discovery service not running." ) - raise FileNotFoundError(e.strerror, e.filename) from e - except Exception as e: + raise + except Exception: _logger.exception("Error in registering with discovery service.") - raise Exception(type(e).__name__, e.args[0]) from e + raise return True def unregister_service(self) -> bool: @@ -172,15 +172,15 @@ def unregister_service(self) -> bool: ) else: _logger.exception("Error in unregistering with discovery service.") - raise Exception(type(e).__name__, e.args[0]) from e - except FileNotFoundError as e: + raise + except FileNotFoundError: _logger.error( "Unable to unregister with discovery service. Possible reason: discovery service not running." ) - raise FileNotFoundError(e.strerror, e.filename) from e - except Exception as e: + raise + except Exception: _logger.exception("Error in unregistering with discovery service.") - raise Exception(type(e).__name__, e.args[0]) from e + raise return True def resolve_service(self, provided_interface: str, service_class: str = "") -> ServiceLocation: From 3c1ed92bc7d2cc21a8102b65031e0e842986017e Mon Sep 17 00:00:00 2001 From: Mounika Battu Date: Mon, 21 Aug 2023 23:52:42 +0530 Subject: [PATCH 43/47] Fix: Don't catch the inner exceptions --- .../_internal/service_manager.py | 69 +++++++++---------- .../measurement/service.py | 13 ++-- 2 files changed, 38 insertions(+), 44 deletions(-) diff --git a/ni_measurementlink_service/_internal/service_manager.py b/ni_measurementlink_service/_internal/service_manager.py index d9dda0173..2ce93e220 100644 --- a/ni_measurementlink_service/_internal/service_manager.py +++ b/ni_measurementlink_service/_internal/service_manager.py @@ -73,42 +73,39 @@ def start( int: The port number of the server """ - try: - self.server = grpc.server( - logging_pool.pool(max_workers=10), - options=[ - ("grpc.max_receive_message_length", -1), - ("grpc.max_send_message_length", -1), - ], - ) - servicer_v1 = MeasurementServiceServicerV1( - measurement_info, - configuration_parameter_list, - output_parameter_list, - measure_function, - ) - v1_measurement_service_pb2_grpc.add_MeasurementServiceServicer_to_server( - servicer_v1, self.server - ) - servicer_v2 = MeasurementServiceServicerV2( - measurement_info, - configuration_parameter_list, - output_parameter_list, - measure_function, - ) - v2_measurement_service_pb2_grpc.add_MeasurementServiceServicer_to_server( - servicer_v2, self.server - ) - port = str(self.server.add_insecure_port("[::]:0")) - self.server.start() - _logger.info("Measurement service hosted on port: %s", port) - self.discovery_client.register_measurement_service(port, service_info, measurement_info) - - self.port = port - - return port - except Exception as e: - raise e + self.server = grpc.server( + logging_pool.pool(max_workers=10), + options=[ + ("grpc.max_receive_message_length", -1), + ("grpc.max_send_message_length", -1), + ], + ) + servicer_v1 = MeasurementServiceServicerV1( + measurement_info, + configuration_parameter_list, + output_parameter_list, + measure_function, + ) + v1_measurement_service_pb2_grpc.add_MeasurementServiceServicer_to_server( + servicer_v1, self.server + ) + servicer_v2 = MeasurementServiceServicerV2( + measurement_info, + configuration_parameter_list, + output_parameter_list, + measure_function, + ) + v2_measurement_service_pb2_grpc.add_MeasurementServiceServicer_to_server( + servicer_v2, self.server + ) + port = str(self.server.add_insecure_port("[::]:0")) + self.server.start() + _logger.info("Measurement service hosted on port: %s", port) + self.discovery_client.register_measurement_service(port, service_info, measurement_info) + + self.port = port + + return port def stop(self) -> None: """Close the Service after un-registering with discovery service and cleanups.""" diff --git a/ni_measurementlink_service/measurement/service.py b/ni_measurementlink_service/measurement/service.py index fbe4887f6..f3d0ca2e9 100644 --- a/ni_measurementlink_service/measurement/service.py +++ b/ni_measurementlink_service/measurement/service.py @@ -373,19 +373,16 @@ def host_service(self) -> MeasurementService: Exception: If register measurement methods not available. """ - try: - if self.measure_function is None: - raise Exception("Error, must register measurement method.") - self.grpc_service.start( + if self.measure_function is None: + raise Exception("Error, must register measurement method.") + self.grpc_service.start( self.measurement_info, self.service_info, self.configuration_parameter_list, self.output_parameter_list, self.measure_function, - ) - return self - except Exception as e: - raise e + ) + return self def _make_annotations_dict( self, From f0decea24a4ad25d59a5e15f9ba6a99df1ededaa Mon Sep 17 00:00:00 2001 From: Mounika Battu Date: Mon, 21 Aug 2023 23:53:09 +0530 Subject: [PATCH 44/47] Fix: FakeDiscoveryServiceError --- tests/integration/test_service_manager.py | 3 ++- tests/utilities/fake_discovery_service.py | 9 +++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_service_manager.py b/tests/integration/test_service_manager.py index d0f196dbf..44fc379c6 100644 --- a/tests/integration/test_service_manager.py +++ b/tests/integration/test_service_manager.py @@ -15,6 +15,7 @@ ) from tests.utilities import loopback_measurement from tests.utilities.fake_discovery_service import ( + FakeDiscoveryServiceError, FakeDiscoveryServiceStub, FakeDiscoveryServiceStubError, ) @@ -50,7 +51,7 @@ def test___grpc_service_without_discovery_service___start_service___service_host def test___grpc_service___start_service_error_registering_measurement___raises_error( grpc_service: GrpcService, ): - with pytest.raises(Exception): + with pytest.raises(FakeDiscoveryServiceError): grpc_service.start( loopback_measurement.measurement_service.measurement_info, loopback_measurement.measurement_service.service_info, diff --git a/tests/utilities/fake_discovery_service.py b/tests/utilities/fake_discovery_service.py index 763e25cf0..637ce1eae 100644 --- a/tests/utilities/fake_discovery_service.py +++ b/tests/utilities/fake_discovery_service.py @@ -29,8 +29,13 @@ class FakeDiscoveryServiceStubError(FakeDiscoveryServiceStub): def RegisterService(self, request): # noqa: N802 - function name should be lowercase """Fake gRPC registration call to discovery service.""" - raise Exception("TestException") + raise FakeDiscoveryServiceError("TestException") def UnregisterService(self, request): # noqa: N802 - function name should be lowercase """Fake gRPC un-registration call to discovery service.""" - raise Exception("TestException") + raise FakeDiscoveryServiceError("TestException") + + +class FakeDiscoveryServiceError(Exception): + """Fake discovery service error to mimic the exceptions thrown from discovery service.""" + pass \ No newline at end of file From 1ab02bf555b081cc9305472e561057651af18a83 Mon Sep 17 00:00:00 2001 From: Mounika Battu Date: Mon, 21 Aug 2023 23:54:37 +0530 Subject: [PATCH 45/47] Fix: Lint errors --- ni_measurementlink_service/measurement/service.py | 10 +++++----- tests/utilities/fake_discovery_service.py | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/ni_measurementlink_service/measurement/service.py b/ni_measurementlink_service/measurement/service.py index f3d0ca2e9..139cee3a3 100644 --- a/ni_measurementlink_service/measurement/service.py +++ b/ni_measurementlink_service/measurement/service.py @@ -376,11 +376,11 @@ def host_service(self) -> MeasurementService: if self.measure_function is None: raise Exception("Error, must register measurement method.") self.grpc_service.start( - self.measurement_info, - self.service_info, - self.configuration_parameter_list, - self.output_parameter_list, - self.measure_function, + self.measurement_info, + self.service_info, + self.configuration_parameter_list, + self.output_parameter_list, + self.measure_function, ) return self diff --git a/tests/utilities/fake_discovery_service.py b/tests/utilities/fake_discovery_service.py index 637ce1eae..d8f06d9c2 100644 --- a/tests/utilities/fake_discovery_service.py +++ b/tests/utilities/fake_discovery_service.py @@ -38,4 +38,5 @@ def UnregisterService(self, request): # noqa: N802 - function name should be lo class FakeDiscoveryServiceError(Exception): """Fake discovery service error to mimic the exceptions thrown from discovery service.""" - pass \ No newline at end of file + + pass From c359015e0c533cceec9f4c62b9bc0f28efc191fe Mon Sep 17 00:00:00 2001 From: Mounika Battu Date: Mon, 21 Aug 2023 23:56:45 +0530 Subject: [PATCH 46/47] Fix: Don't catch inner exceptions in stop method --- ni_measurementlink_service/_internal/service_manager.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/ni_measurementlink_service/_internal/service_manager.py b/ni_measurementlink_service/_internal/service_manager.py index 2ce93e220..260c79e73 100644 --- a/ni_measurementlink_service/_internal/service_manager.py +++ b/ni_measurementlink_service/_internal/service_manager.py @@ -109,9 +109,6 @@ def start( def stop(self) -> None: """Close the Service after un-registering with discovery service and cleanups.""" - try: - self.discovery_client.unregister_service() - self.server.stop(5) - _logger.info("Measurement service closed.") - except Exception as e: - raise Exception(e) + self.discovery_client.unregister_service() + self.server.stop(5) + _logger.info("Measurement service closed.") From 1185d8d9400e781d2302d893bc17b3253939dd39 Mon Sep 17 00:00:00 2001 From: Mounika Battu Date: Mon, 21 Aug 2023 23:57:48 +0530 Subject: [PATCH 47/47] Fix: Spacing --- ni_measurementlink_service/_internal/service_manager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ni_measurementlink_service/_internal/service_manager.py b/ni_measurementlink_service/_internal/service_manager.py index 260c79e73..5700bc47d 100644 --- a/ni_measurementlink_service/_internal/service_manager.py +++ b/ni_measurementlink_service/_internal/service_manager.py @@ -104,7 +104,6 @@ def start( self.discovery_client.register_measurement_service(port, service_info, measurement_info) self.port = port - return port def stop(self) -> None: