Skip to content

Commit

Permalink
Merge branch 'main' of https://github.com/ni/measurementlink-python i…
Browse files Browse the repository at this point in the history
…nto users/vikram/fix-disc-service-resource-warning
  • Loading branch information
Vikram Avudaiappan committed Jul 31, 2023
2 parents 4af1b55 + c89b733 commit 49a2a81
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 16 deletions.
10 changes: 9 additions & 1 deletion ni_measurementlink_service/_internal/discovery_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ def register_measurement_service(
_logger.error(
"Unable to register with discovery service. Possible reason: discovery service not running."
)
return False
except Exception:
_logger.exception("Error in registering with discovery service.")
return False
Expand All @@ -163,6 +164,7 @@ def unregister_service(self) -> bool:
_logger.info("Successfully unregistered with discovery service.")
else:
_logger.info("Not registered with discovery service.")
return False
except grpc.RpcError as e:
if e.code() == grpc.StatusCode.UNAVAILABLE:
_logger.error(
Expand All @@ -175,6 +177,7 @@ def unregister_service(self) -> bool:
_logger.error(
"Unable to unregister with discovery service. Possible reason: discovery service not running."
)
return False
except Exception:
_logger.exception("Error in unregistering with discovery service.")
return False
Expand Down Expand Up @@ -257,7 +260,12 @@ def _key_file_exists(key_file_path: pathlib.Path) -> bool:
def _start_service(exe_file_path: pathlib.PurePath, key_file_path: pathlib.Path) -> None:
"""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)
_discovery_service_subprocess = subprocess.Popen(
[exe_file_path],
cwd=exe_file_path.parent,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)
# After the execution of process, check for key file existence in the path
# stop checking after 30 seconds have elapsed and throw error
timeout_time = time.time() + _START_SERVICE_TIMEOUT
Expand Down
52 changes: 37 additions & 15 deletions tests/unit/test_discovery_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"""
import json
import pathlib
import subprocess
from typing import cast

import pytest
Expand Down Expand Up @@ -76,19 +77,7 @@ def test___discovery_service_available___unregister_non_registered_service___unr
):
unregistration_success_flag = discovery_client.unregister_service()

assert ~unregistration_success_flag # False


def test___discovery_service_unavailable___register_service_registration_failure(
discovery_client: DiscoveryClient,
):
discovery_client.register_measurement_service(
_TEST_SERVICE_PORT, _TEST_SERVICE_INFO, _TEST_MEASUREMENT_INFO
)

unregistration_success_flag = discovery_client.unregister_service()

assert ~unregistration_success_flag # False
assert not unregistration_success_flag


def test___get_discovery_service_address___start_service_jit___returns_expected_value(
Expand All @@ -115,7 +104,12 @@ def test___get_discovery_service_address___start_service_jit___returns_expected_

exe_file_path = temp_directory / _MOCK_REGISTRATION_FILE_CONTENT["discovery"]["path"]

mock_popen.assert_called_once_with([exe_file_path], cwd=exe_file_path.parent)
mock_popen.assert_called_once_with(
[exe_file_path],
cwd=exe_file_path.parent,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)
assert _TEST_SERVICE_PORT in discovery_service_address


Expand Down Expand Up @@ -158,7 +152,35 @@ def test___start_discovery_service___key_file_exist_after_poll___service_start_s

_start_service(exe_file_path, temp_discovery_key_file_path)

mock_popen.assert_called_once_with([exe_file_path], cwd=exe_file_path.parent)
mock_popen.assert_called_once_with(
[exe_file_path],
cwd=exe_file_path.parent,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)


def test___discovery_service_exe_unavailable___register_service___registration_failure(
mocker: MockerFixture,
temp_discovery_key_file_path: pathlib.Path,
temp_registration_json_file_path: pathlib.Path,
):
mocker.patch(
"ni_measurementlink_service._internal.discovery_client._get_key_file_path",
return_value=temp_discovery_key_file_path,
)
mocker.patch(
"ni_measurementlink_service._internal.discovery_client._get_registration_json_file_path",
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


@pytest.fixture
Expand Down

0 comments on commit 49a2a81

Please sign in to comment.