Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Adding optional override for adaptor request timeout default to EntryPoint start method #149

Merged
merged 3 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/openjd/adaptor_runtime/_background/frontend_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
HeartbeatResponse,
)

_FRONTEND_RUNNER_REQUEST_TIMEOUT: float = 5.0

if OSName.is_windows():
from ...adaptor_runtime_client.named_pipe.named_pipe_helper import NamedPipeHelper
import pywintypes
Expand All @@ -57,7 +59,7 @@ class FrontendRunner:
def __init__(
self,
*,
timeout_s: float = 5.0,
timeout_s: float = _FRONTEND_RUNNER_REQUEST_TIMEOUT,
heartbeat_interval: float = 1.0,
connection_settings: ConnectionSettings | None = None,
) -> None:
Expand Down
18 changes: 14 additions & 4 deletions src/openjd/adaptor_runtime/_entrypoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

from .adaptors import AdaptorRunner, BaseAdaptor
from ._background import BackendRunner, FrontendRunner, InMemoryLogBuffer, LogBufferHandler
from ._background.frontend_runner import _FRONTEND_RUNNER_REQUEST_TIMEOUT
from ._background.loaders import (
ConnectionSettingsFileLoader,
ConnectionSettingsEnvLoader,
Expand Down Expand Up @@ -246,12 +247,18 @@ def _get_integration_data(self, parsed_args: Namespace) -> _IntegrationData:
),
)

def start(self, reentry_exe: Optional[Path] = None) -> None:
def start(
self,
reentry_exe: Optional[Path] = None,
timeout_in_seconds: float = _FRONTEND_RUNNER_REQUEST_TIMEOUT,
) -> None:
Dismissed Show dismissed Hide dismissed
"""
Starts the run of the adaptor.

Args:
reentry_exe (Path): The path to the binary executable that for adaptor reentry.
timeout_in_seconds (float): The maximum time in seconds to wait for data before
raising a TimeoutError. Defaults to 5 seconds. None means waiting indefinitely.
"""
parser, parsed_args = self._parse_args()
log_config = self._init_loggers(
Expand Down Expand Up @@ -300,7 +307,7 @@ def start(self, reentry_exe: Optional[Path] = None) -> None:
return self._handle_run(adaptor, integration_data)
elif parsed_args.command == "daemon": # pragma: no branch
return self._handle_daemon(
adaptor, parsed_args, log_config, integration_data, reentry_exe
adaptor, parsed_args, log_config, integration_data, timeout_in_seconds, reentry_exe
)

def _handle_is_compatible(
Expand Down Expand Up @@ -367,6 +374,7 @@ def _handle_daemon(
parsed_args: _ParsedArgs,
log_config: _LogConfig,
integration_data: _IntegrationData,
timeout_in_seconds: float,
reentry_exe: Optional[Path] = None,
):
# Validate args
Expand Down Expand Up @@ -408,7 +416,7 @@ def _handle_daemon(
# This process is running in frontend mode. Create the frontend runner and send
# the appropriate request to the backend.
if subcommand == "start":
frontend = FrontendRunner()
frontend = FrontendRunner(timeout_s=timeout_in_seconds)
adaptor_module = sys.modules.get(self.adaptor_class.__module__)
if adaptor_module is None:
raise ModuleNotFoundError(
Expand All @@ -435,7 +443,9 @@ def _handle_daemon(
else ConnectionSettingsEnvLoader()
)
conn_settings = conn_settings_loader.load()
frontend = FrontendRunner(connection_settings=conn_settings)
frontend = FrontendRunner(
connection_settings=conn_settings, timeout_s=timeout_in_seconds
)
if subcommand == "run":
frontend.run(integration_data.run_data)
elif subcommand == "stop":
Expand Down
67 changes: 62 additions & 5 deletions test/openjd/adaptor_runtime/unit/test_entrypoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
)
from openjd.adaptor_runtime.adaptors import BaseAdaptor, SemanticVersion
from openjd.adaptor_runtime._background import BackendRunner, FrontendRunner
from openjd.adaptor_runtime._background.frontend_runner import _FRONTEND_RUNNER_REQUEST_TIMEOUT
from openjd.adaptor_runtime._background.model import ConnectionSettings
from openjd.adaptor_runtime._osname import OSName
from openjd.adaptor_runtime._entrypoint import _load_data
Expand Down Expand Up @@ -564,7 +565,7 @@ def test_background_start_raises_when_adaptor_module_not_loaded(

# THEN
assert raised_err.match(f"Adaptor module is not loaded: {FakeAdaptor.__module__}")
mock_magic_init.assert_called_once_with()
mock_magic_init.assert_called_once_with(timeout_s=_FRONTEND_RUNNER_REQUEST_TIMEOUT)

@pytest.mark.parametrize(
argnames="reentry_exe",
Expand Down Expand Up @@ -606,7 +607,7 @@ def test_runs_background_start(
entrypoint.start(reentry_exe=reentry_exe)

# THEN
mock_magic_init.assert_called_once_with()
mock_magic_init.assert_called_once_with(timeout_s=_FRONTEND_RUNNER_REQUEST_TIMEOUT)
mock_init.assert_called_once_with(
adaptor_module=mock_adaptor_module,
connection_file_path=conn_file.resolve(),
Expand Down Expand Up @@ -647,7 +648,9 @@ def test_runs_background_stop(
entrypoint.start()

# THEN
mock_magic_init.assert_called_once_with(connection_settings=connection_settings)
mock_magic_init.assert_called_once_with(
connection_settings=connection_settings, timeout_s=_FRONTEND_RUNNER_REQUEST_TIMEOUT
)
mock_end.assert_called_once()
mock_shutdown.assert_called_once_with()

Expand Down Expand Up @@ -683,7 +686,9 @@ def test_runs_background_run(
entrypoint.start()

# THEN
mock_magic_init.assert_called_once_with(connection_settings=conn_settings)
mock_magic_init.assert_called_once_with(
connection_settings=conn_settings, timeout_s=_FRONTEND_RUNNER_REQUEST_TIMEOUT
)
mock_run.assert_called_once_with(run_data)
mock_connection_settings_load.assert_called_once()

Expand Down Expand Up @@ -722,6 +727,57 @@ def test_background_no_signal_hook(
# THEN
signal_mock.assert_not_called()

@pytest.mark.parametrize(
argnames="reentry_exe",
argvalues=[
(None,),
(Path("reeentry_exe_value"),),
],
)
@patch.object(FrontendRunner, "__init__", return_value=None)
@patch.object(FrontendRunner, "init")
@patch.object(FrontendRunner, "start")
def test_frontend_runner_timeout_override(
self,
mock_start: MagicMock,
mock_init: MagicMock,
mock_magic_init: MagicMock,
reentry_exe: Optional[Path],
):
# GIVEN
test_timeout_override = 600
conn_file = Path(os.sep) / "path" / "to" / "conn_file"
with patch.object(
runtime_entrypoint.sys,
"argv",
[
"Adaptor",
"daemon",
"start",
"--connection-file",
str(conn_file),
],
):
mock_adaptor_module = Mock()
entrypoint = EntryPoint(FakeAdaptor)

# WHEN
with patch.dict(
runtime_entrypoint.sys.modules, {FakeAdaptor.__module__: mock_adaptor_module}
):
entrypoint.start(reentry_exe=reentry_exe, timeout_in_seconds=test_timeout_override)

# THEN
mock_magic_init.assert_called_once_with(timeout_s=test_timeout_override)
mock_init.assert_called_once_with(
adaptor_module=mock_adaptor_module,
connection_file_path=conn_file.resolve(),
init_data={},
path_mapping_data={},
reentry_exe=reentry_exe,
)
mock_start.assert_called_once_with()

@patch.object(runtime_entrypoint, "ConnectionSettingsFileLoader")
@patch.object(runtime_entrypoint, "FrontendRunner")
def test_makes_connection_file_path_absolute(
Expand Down Expand Up @@ -761,7 +817,8 @@ def test_makes_connection_file_path_absolute(
mock_absolute.assert_any_call()
mock_connection_settings_loader.assert_called_once_with(mock_absolute.return_value)
mock_runner.assert_called_once_with(
connection_settings=mock_connection_settings_loader.return_value.load.return_value
connection_settings=mock_connection_settings_loader.return_value.load.return_value,
timeout_s=_FRONTEND_RUNNER_REQUEST_TIMEOUT,
)


Expand Down