From 55215b5f2cc01719015ee5d48b3676b85f97421c Mon Sep 17 00:00:00 2001 From: Brian Axelson <86568017+baxeaz@users.noreply.github.com> Date: Wed, 16 Oct 2024 20:33:13 -0700 Subject: [PATCH 1/3] feat: Adding optional override for adaptor request timeout default to EntryPoint start method Signed-off-by: unknown <86568017+baxeaz@users.noreply.github.com> --- .../_background/frontend_runner.py | 4 +- src/openjd/adaptor_runtime/_entrypoint.py | 18 +++-- .../adaptor_runtime/unit/test_entrypoint.py | 67 +++++++++++++++++-- 3 files changed, 79 insertions(+), 10 deletions(-) diff --git a/src/openjd/adaptor_runtime/_background/frontend_runner.py b/src/openjd/adaptor_runtime/_background/frontend_runner.py index 1b229cb..c83b4ba 100644 --- a/src/openjd/adaptor_runtime/_background/frontend_runner.py +++ b/src/openjd/adaptor_runtime/_background/frontend_runner.py @@ -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 @@ -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: diff --git a/src/openjd/adaptor_runtime/_entrypoint.py b/src/openjd/adaptor_runtime/_entrypoint.py index e7a112d..8127390 100644 --- a/src/openjd/adaptor_runtime/_entrypoint.py +++ b/src/openjd/adaptor_runtime/_entrypoint.py @@ -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, @@ -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: Optional[float] = _FRONTEND_RUNNER_REQUEST_TIMEOUT, + ) -> None: """ Starts the run of the adaptor. Args: reentry_exe (Path): The path to the binary executable that for adaptor reentry. + timeout_in_seconds (Optional[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( @@ -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( @@ -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 @@ -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( @@ -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": diff --git a/test/openjd/adaptor_runtime/unit/test_entrypoint.py b/test/openjd/adaptor_runtime/unit/test_entrypoint.py index 80de3dc..46adc5d 100644 --- a/test/openjd/adaptor_runtime/unit/test_entrypoint.py +++ b/test/openjd/adaptor_runtime/unit/test_entrypoint.py @@ -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 @@ -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", @@ -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(), @@ -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() @@ -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() @@ -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( @@ -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, ) From 361507419b47777aa768aa7a91cccb076354d44c Mon Sep 17 00:00:00 2001 From: Brian Axelson <86568017+baxeaz@users.noreply.github.com> Date: Wed, 23 Oct 2024 06:28:54 -0700 Subject: [PATCH 2/3] fix: Removing optional from timeout_in_seconds Signed-off-by: Brian Axelson <86568017+baxeaz@users.noreply.github.com> --- src/openjd/adaptor_runtime/_entrypoint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openjd/adaptor_runtime/_entrypoint.py b/src/openjd/adaptor_runtime/_entrypoint.py index 8127390..1bf9986 100644 --- a/src/openjd/adaptor_runtime/_entrypoint.py +++ b/src/openjd/adaptor_runtime/_entrypoint.py @@ -250,7 +250,7 @@ def _get_integration_data(self, parsed_args: Namespace) -> _IntegrationData: def start( self, reentry_exe: Optional[Path] = None, - timeout_in_seconds: Optional[float] = _FRONTEND_RUNNER_REQUEST_TIMEOUT, + timeout_in_seconds: float = _FRONTEND_RUNNER_REQUEST_TIMEOUT, ) -> None: """ Starts the run of the adaptor. From c64fdbec9d6e9063559b5e813499209759b38fe5 Mon Sep 17 00:00:00 2001 From: Brian Axelson <86568017+baxeaz@users.noreply.github.com> Date: Wed, 23 Oct 2024 06:44:45 -0700 Subject: [PATCH 3/3] docs: Removing Optional from comments Signed-off-by: Brian Axelson <86568017+baxeaz@users.noreply.github.com> --- src/openjd/adaptor_runtime/_entrypoint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openjd/adaptor_runtime/_entrypoint.py b/src/openjd/adaptor_runtime/_entrypoint.py index 1bf9986..23ad98d 100644 --- a/src/openjd/adaptor_runtime/_entrypoint.py +++ b/src/openjd/adaptor_runtime/_entrypoint.py @@ -257,7 +257,7 @@ def start( Args: reentry_exe (Path): The path to the binary executable that for adaptor reentry. - timeout_in_seconds (Optional[float]): The maximum time in seconds to wait for data before + 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()