Skip to content

Commit

Permalink
feat: Add the ability for the adaptor to specify its reentry executab…
Browse files Browse the repository at this point in the history
…le (#69)

- Also remove the relative import to _OSName and just use os.name so
  that it is compatible with how we inject the module into DCCs.

Signed-off-by: Mark Wiebe <399551+mwiebe@users.noreply.github.com>
  • Loading branch information
mwiebe authored Feb 8, 2024
1 parent 8d8e19f commit 9647ec8
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 24 deletions.
36 changes: 23 additions & 13 deletions src/openjd/adaptor_runtime/_background/frontend_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import subprocess
import sys
import time
from pathlib import Path
import urllib.parse as urllib_parse
from threading import Event
from types import FrameType
Expand Down Expand Up @@ -70,6 +71,7 @@ def init(
adaptor_module: ModuleType,
init_data: dict | None = None,
path_mapping_data: dict | None = None,
reentry_exe: Path | None = None,
) -> None:
"""
Creates the backend process then sends a heartbeat request to verify that it has started
Expand All @@ -79,6 +81,7 @@ def init(
adaptor_module (ModuleType): The module of the adaptor running the runtime.
init_data (dict): Data to pass to the adaptor during initialization.
path_mapping_data (dict): Path mapping rules to make available to the adaptor while it's running.
reentry_exe (Path): The path to the binary executable that for adaptor reentry.
"""
if adaptor_module.__package__ is None:
raise Exception(f"Adaptor module is not a package: {adaptor_module}")
Expand All @@ -96,19 +99,26 @@ def init(
path_mapping_data = {}

_logger.info("Initializing backend process...")
args = [
sys.executable,
"-m",
adaptor_module.__package__,
"daemon",
"_serve",
"--connection-file",
self._connection_file_path,
"--init-data",
json.dumps(init_data),
"--path-mapping-rules",
json.dumps(path_mapping_data),
]
if reentry_exe is None:
args = [
sys.executable,
"-m",
adaptor_module.__package__,
]
else:
args = [str(reentry_exe)]
args.extend(
[
"daemon",
"_serve",
"--connection-file",
self._connection_file_path,
"--init-data",
json.dumps(init_data),
"--path-mapping-rules",
json.dumps(path_mapping_data),
]
)
try:
process = subprocess.Popen(
args,
Expand Down
8 changes: 6 additions & 2 deletions src/openjd/adaptor_runtime/_entrypoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import os
import signal
import sys
from pathlib import Path
from argparse import ArgumentParser, Namespace
from types import FrameType as FrameType
from typing import TYPE_CHECKING, Any, Optional, Type, TypeVar
Expand Down Expand Up @@ -92,9 +93,12 @@ def __init__(self, adaptor_class: Type[_U]) -> None:
# 'background' command
self._adaptor_runner: Optional[AdaptorRunner] = None

def start(self) -> None:
def start(self, reentry_exe: Optional[Path] = None) -> None:
"""
Starts the run of the adaptor.
Args:
reentry_exe (Path): The path to the binary executable that for adaptor reentry.
"""
formatter = ConditionalFormatter(
"%(levelname)s: %(message)s", ignore_patterns=[_OPENJD_LOG_REGEX]
Expand Down Expand Up @@ -214,7 +218,7 @@ def start(self) -> None:
f"Adaptor module is not loaded: {self.adaptor_class.__module__}"
)

frontend.init(adaptor_module, init_data, path_mapping_data)
frontend.init(adaptor_module, init_data, path_mapping_data, reentry_exe)
frontend.start()
elif subcommand == "run":
frontend.run(run_data)
Expand Down
5 changes: 3 additions & 2 deletions src/openjd/adaptor_runtime_client/__init__.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.

import os

from .action import Action
from .base_client_interface import (
PathMappingRule,
)
from ..adaptor_runtime._osname import OSName

if OSName.is_posix():
if os.name == "posix":
from .posix_client_interface import HTTPClientInterface as ClientInterface

# This is just for backward compatible
Expand Down
4 changes: 4 additions & 0 deletions src/openjd/adaptor_runtime_client/win_client_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@


from .base_client_interface import BaseClientInterface

# TODO: We need to remove this relative import, since it won't work with how
# we inject this module into DCCs. Likely by copying the function
# NamedPipeHelper.send_named_pipe_request into this adaptor_runtime_client namespace.
from ..adaptor_runtime._named_pipe.named_pipe_helper import NamedPipeHelper

_DEFAULT_TIMEOUT_IN_SECONDS = 15
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
import signal
import subprocess
import sys
from pathlib import Path
from types import ModuleType
from typing import Generator
from typing import Generator, Optional
from unittest.mock import MagicMock, PropertyMock, call, mock_open, patch

import pytest
Expand Down Expand Up @@ -53,6 +54,13 @@ class TestInit:
Tests for the FrontendRunner.init method
"""

@pytest.mark.parametrize(
argnames=("reentry_exe"),
argvalues=[
(None,),
(Path("reeentry_exe_value"),),
],
)
@patch.object(frontend_runner.sys, "argv")
@patch.object(frontend_runner.sys, "executable")
@patch.object(frontend_runner.json, "dumps")
Expand All @@ -70,6 +78,7 @@ def test_initializes_backend_process(
mock_sys_executable: MagicMock,
mock_sys_argv: MagicMock,
caplog: pytest.LogCaptureFixture,
reentry_exe: Optional[Path],
):
# GIVEN
caplog.set_level("DEBUG")
Expand All @@ -87,7 +96,7 @@ def test_initializes_backend_process(
runner = FrontendRunner(conn_file_path)

# WHEN
runner.init(adaptor_module, init_data, path_mapping_data)
runner.init(adaptor_module, init_data, path_mapping_data, reentry_exe)

# THEN
assert caplog.messages == [
Expand All @@ -97,8 +106,8 @@ def test_initializes_backend_process(
"Connected successfully",
]
mock_exists.assert_called_once_with(conn_file_path)
mock_Popen.assert_called_once_with(
[
if reentry_exe is None:
expected_args = [
sys.executable,
"-m",
adaptor_module.__package__,
Expand All @@ -110,7 +119,21 @@ def test_initializes_backend_process(
json.dumps(init_data),
"--path-mapping-rules",
json.dumps(path_mapping_data),
],
]
else:
expected_args = [
str(reentry_exe),
"daemon",
"_serve",
"--connection-file",
conn_file_path,
"--init-data",
json.dumps(init_data),
"--path-mapping-rules",
json.dumps(path_mapping_data),
]
mock_Popen.assert_called_once_with(
expected_args,
shell=False,
close_fds=True,
start_new_session=True,
Expand Down
14 changes: 12 additions & 2 deletions test/openjd/adaptor_runtime/unit/test_entrypoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import argparse
import json
import signal
from pathlib import Path
from typing import Optional
from unittest.mock import ANY, MagicMock, Mock, PropertyMock, mock_open, patch

import jsonschema
Expand Down Expand Up @@ -389,6 +391,13 @@ def test_background_start_raises_when_adaptor_module_not_loaded(
_parse_args_mock.assert_called_once()
mock_magic_init.assert_called_once_with(conn_file)

@pytest.mark.parametrize(
argnames=("reentry_exe"),
argvalues=[
(None,),
(Path("reeentry_exe_value"),),
],
)
@patch.object(EntryPoint, "_parse_args")
@patch.object(FrontendRunner, "__init__", return_value=None)
@patch.object(FrontendRunner, "init")
Expand All @@ -399,6 +408,7 @@ def test_runs_background_start(
mock_magic_init: MagicMock,
mock_magic_start: MagicMock,
_parse_args_mock: MagicMock,
reentry_exe: Optional[Path],
):
# GIVEN
conn_file = "/path/to/conn_file"
Expand All @@ -414,11 +424,11 @@ def test_runs_background_start(
with patch.dict(
runtime_entrypoint.sys.modules, {FakeAdaptor.__module__: mock_adaptor_module}
):
entrypoint.start()
entrypoint.start(reentry_exe=reentry_exe)

# THEN
_parse_args_mock.assert_called_once()
mock_magic_init.assert_called_once_with(mock_adaptor_module, {}, {})
mock_magic_init.assert_called_once_with(mock_adaptor_module, {}, {}, reentry_exe)
mock_magic_start.assert_called_once_with(conn_file)
mock_start.assert_called_once_with()

Expand Down

0 comments on commit 9647ec8

Please sign in to comment.