Skip to content

Commit

Permalink
refactor: Update comments and remove unnecessary parentheses (#100)
Browse files Browse the repository at this point in the history
Signed-off-by: Hongli Chen <[email protected]>
  • Loading branch information
Honglichenn authored Mar 28, 2024
1 parent ec0cda1 commit 0adc478
Show file tree
Hide file tree
Showing 19 changed files with 66 additions and 53 deletions.
2 changes: 1 addition & 1 deletion hatch_version_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def _prepare(self) -> bool:
if missing_required_opts:
_logger.warn(
f"Required options {missing_required_opts} are missing or empty. "
"Contining without copying sources to destinations...",
"Continuing without copying sources to destinations...",
file=sys.stderr,
)
return False
Expand Down
16 changes: 12 additions & 4 deletions src/openjd/adaptor_runtime/_background/backend_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,18 @@ def __init__(
signal.signal(signal.SIGBREAK, self._sigint_handler) # type: ignore[attr-defined]

def _sigint_handler(self, signum: int, frame: Optional[FrameType]) -> None:
"""Signal handler that is invoked when the process receives a SIGINT/SIGTERM"""
_logger.info("Interruption signal recieved.")
# OpenJD dictates that a SIGTERM/SIGINT results in a cancel workflow being
# kicked off.
"""
Signal handler for interrupt signals.
This handler is invoked when the process receives a SIGTERM signal on Linux and SIGBREAK signal on Windows.
It calls the cancel method on the adaptor runner to initiate cancellation workflow.
Args:
signum: The number of the received signal.
frame: The current stack frame.
"""
_logger.info("Interruption signal received.")
# Open Job Description dictates that an interrupt signal should trigger cancellation
if self._server is not None:
ServerResponseGenerator.submit_task(
self._server, self._adaptor_runner._cancel, force_immediate=True
Expand Down
20 changes: 15 additions & 5 deletions src/openjd/adaptor_runtime/_background/frontend_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ def _send_linux_request(
finally:
conn.close()

if response.status >= 400 and response.status < 600:
if 400 <= response.status < 600:
errmsg = f"Received unexpected HTTP status code {response.status}: {response.reason}"
_logger.error(errmsg)
raise HTTPError(response, errmsg)
Expand All @@ -321,10 +321,20 @@ def connection_settings(self) -> ConnectionSettings:
return self._connection_settings

def _sigint_handler(self, signum: int, frame: Optional[FrameType]) -> None:
"""Signal handler that is invoked when the process receives a SIGINT/SIGTERM"""
_logger.info("Interruption signal recieved.")
# OpenJD dictates that a SIGTERM/SIGINT results in a cancel workflow being
# kicked off.
"""
Signal handler for interrupt signals.
This handler is invoked when the process receives a SIGTERM signal on Linux and SIGBREAK signal on Windows.
It calls the cancel method on the adaptor runner to initiate cancellation workflow.
Args:
signum: The number of the received signal.
frame: The current stack frame.
"""

_logger.info("Interrupt signal received.")

# Open Job Description dictates that an interrupt signal should trigger cancellation
self.cancel()


Expand Down
10 changes: 5 additions & 5 deletions src/openjd/adaptor_runtime/_entrypoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
"This can be a JSON string or the path to a file containing a JSON string in the format "
"file://path/to/file.json"
),
"show_config": ("Prints the adaptor runtime configuration, then the program exits."),
"show_config": "Prints the adaptor runtime configuration, then the program exits.",
"connection_file": "The file path to the connection file for use in background mode.",
}

Expand Down Expand Up @@ -109,7 +109,7 @@ def has_compatibility_with(self, expected: "_VersionInfo") -> bool:
expected by something like a job template.
Args:
other (_VersionInfo): The VersionInfo to compare with.
expected (_VersionInfo): The VersionInfo to compare with.
"""
return self.adaptor_cli_version.has_compatibility_with(
expected.adaptor_cli_version
Expand Down Expand Up @@ -436,10 +436,10 @@ def _build_argparser(self) -> ArgumentParser:
return parser

def _sigint_handler(self, signum: int, frame: Optional[FrameType]) -> None:
"""Signal handler that is invoked when the process receives a SIGINT/SIGTERM"""
"""Signal handler that is invoked when the process receives a SIGINT/SIGTERM in Linux and SIGBREAK in Windows"""
if self._adaptor_runner is not None:
_logger.info("Interruption signal recieved.")
# OpenJD dictates that a SIGTERM/SIGINT results in a cancel workflow being
_logger.info("Interruption signal received.")
# OpenJD dictates that an Interruption signal results in a cancel workflow being
# kicked off.
self._adaptor_runner._cancel()

Expand Down
4 changes: 2 additions & 2 deletions src/openjd/adaptor_runtime/_utils/_secure_open.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ def secure_open(
OWNER read/write bit-wise OR'd with the mask argument provided
If the open_mode only involves reading the file, the permissions are not changed.
Args:
file (StrOrBytesPath): The path to the file to open
mode (str): The string mode for opening the file. A combination of r, w, a, x, +
path (StrOrBytesPath): The path to the file to open
open_mode (str): The string mode for opening the file. A combination of r, w, a, x, +
encoding (str, optional): The encoding of the file to open. Defaults to None.
newline (str, optional): The newline character to use. Defaults to None.
mask (int, optional): Additional masks to apply to the opened file. Defaults to 0.
Expand Down
2 changes: 1 addition & 1 deletion src/openjd/adaptor_runtime/adaptors/_base_adaptor.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"BaseAdaptor",
]

# "{ADAPTORNAME}_" is put in front of this variables to make the full env variable
# "{ADAPTORNAME}_" is put in front of these variables to make the full env variable
# ie. MAYAADAPTOR_CONFIG_PATH
_ENV_CONFIG_PATH_TEMPLATE = "CONFIG_PATH"
# Directory containing adaptor schemas
Expand Down
10 changes: 0 additions & 10 deletions src/openjd/adaptor_runtime/adaptors/_path_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,6 @@ def from_dict(*, rule: dict[str, str]) -> PathMappingRule:
if not rule:
raise ValueError("Empty path mapping rule")

# TODO - DELETE ONCE MIGRATION COMPLETE
# The field "source_os" was renamed to "source_path_format", but interfaces may still
# provide the old name until they're updated. Remove once we're sure all interfaces are
# updated.
if "source_os" in rule:
new_rule = dict(**rule)
new_rule["source_path_format"] = new_rule["source_os"]
del new_rule["source_os"]
rule = new_rule
# END TODO
return PathMappingRule(**rule)

def to_dict(self) -> dict[str, str]:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def __init__(self, config: dict) -> None:

def override(self: _T, other: _T) -> _T:
"""
Creates a new Configuration with the configuration values in this object overriden by
Creates a new Configuration with the configuration values in this object overridden by
another configuration.
Args:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def create_adaptor_configuration_manager(
adaptor_name: str,
default_config_path: str,
schema_path: str | List[str] | None = None,
additional_config_paths: list[str] = [],
additional_config_paths: List[str] | None = None,
) -> ConfigurationManager[_AdaptorConfigType]:
"""
Creates a ConfigurationManager for an adaptor.
Expand All @@ -43,7 +43,11 @@ def create_adaptor_configuration_manager(
schema_path (str, List[str], optional): The path(s) to a JSON Schema file(s) to validate
the configuration with. If left as None, only the base adaptor configuration values will be
validated.
additional_config_paths (list[str], Optional): Paths to additional configuration files. These
will have the highest priority and will be applied in the order they are provided.
"""
if additional_config_paths is None:
additional_config_paths = []
schema_paths = [os.path.abspath(os.path.join(_DIR, "_adaptor_configuration.schema.json"))]
if isinstance(schema_path, str):
schema_paths.append(schema_path)
Expand Down Expand Up @@ -118,7 +122,7 @@ def __init__(
system_config_path: str,
user_config_rel_path: str,
schema_path: str | List[str] | None = None,
additional_config_paths: list[str] = [],
additional_config_paths: list[str] | None = None,
) -> None:
"""
Initializes a ConfigurationManager object.
Expand All @@ -132,9 +136,11 @@ def __init__(
schema_path (str, List[str], Optional): The path(s) to the JSON Schema file to use.
If multiple are given then they will be used in the order they are provided.
If none are given then validation will be skipped for configuration files.
additional_config_paths (list[str]): Paths to additional configuration files. These
additional_config_paths (list[str], Optional): Paths to additional configuration files. These
will have the highest priority and will be applied in the order they are provided.
"""
if additional_config_paths is None:
additional_config_paths = []
self._config_cls = config_cls
self._schema_path = schema_path
self._default_config_path = default_config_path
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@


class ActionsQueue:
"""This class will manage the Queue of Actions. This class will be reponsible for
"""This class will manage the Queue of Actions. This class will be responsible for
enqueueing, or dequeueing Actions, and converting actions to and from json strings."""

_actions_queue: Deque[Action]
Expand Down
8 changes: 1 addition & 7 deletions test/openjd/adaptor_runtime/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,7 @@

# List of platforms that can be used to mark tests as specific to that platform
# See [tool.pytest.ini_options] -> markers in pyproject.toml
_PLATFORMS = set(
[
"Linux",
"Windows",
"Darwin",
]
)
_PLATFORMS = {"Linux", "Windows", "Darwin"}


def pytest_runtest_setup(item: pytest.Item):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,10 @@ def test_init(
connection_settings = _load_connection_settings(connection_file_path)

if OSName.is_windows():
try:
import pywintypes
import win32file
import pywintypes
import win32file

try:
handle = win32file.CreateFile(
connection_settings.socket,
win32file.GENERIC_READ,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from openjd.adaptor_runtime import EntryPoint
from openjd.adaptor_runtime._osname import OSName

mod_path = (Path(__file__).parent).resolve()
mod_path = Path(__file__).parent.resolve()
sys.path.append(str(mod_path))
if (_pypath := os.environ.get("PYTHONPATH")) is not None:
os.environ["PYTHONPATH"] = ":".join((_pypath, str(mod_path)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,10 @@ def __init__(
default_config_path="",
system_config_path="",
user_config_rel_path="",
additional_config_paths=[],
additional_config_paths=None,
) -> None:
if additional_config_paths is None:
additional_config_paths = []
super().__init__(
config_cls=Configuration,
schema_path=schema_path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def test_actions_queue_append_start(self) -> None:
assert aq.dequeue_action() is None

def test_len(self) -> None:
"""Testing that our overriden __len__ works as expected."""
"""Testing that our overridden __len__ works as expected."""
aq = _ActionsQueue()

# Starting off with an empty queue.
Expand All @@ -64,7 +64,7 @@ def test_len(self) -> None:
assert len(aq) == 0

def test_bool(self) -> None:
"""Testing that our overriden __bool__ works as expected."""
"""Testing that our overridden __bool__ works as expected."""
aq = _ActionsQueue()

# Starting off with an empty queue.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class TestInit:
"""

@pytest.mark.parametrize(
argnames=("reentry_exe"),
argnames="reentry_exe",
argvalues=[
(None,),
(Path("reeentry_exe_value"),),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def mocked_chunk_id():
with patch.object(LogBuffer, "_create_id") as mock_create_id:
chunk_id = "id"
mock_create_id.return_value = chunk_id
yield (chunk_id, mock_create_id)
yield chunk_id, mock_create_id


class TestInMemoryLogBuffer:
Expand Down
7 changes: 5 additions & 2 deletions test/openjd/adaptor_runtime/unit/test_entrypoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,10 @@ def test_uses_default_config_on_unsupported_system(
mock_get_default_config.assert_called_once()
assert entrypoint.config is mock_get_default_config.return_value
assert f"The current system ({OSName()}) is not supported for runtime "
"configuration. Only the default configuration will be loaded. Full error: " in caplog.text
assert (
"configuration. Only the default configuration will be loaded. Full error: "
in caplog.text
)

@patch.object(ConfigurationManager, "build_config")
@patch.object(RuntimeConfiguration, "config", new_callable=PropertyMock)
Expand Down Expand Up @@ -552,7 +555,7 @@ def test_background_start_raises_when_adaptor_module_not_loaded(
mock_magic_init.assert_called_once_with(conn_file)

@pytest.mark.parametrize(
argnames=("reentry_exe"),
argnames="reentry_exe",
argvalues=[
(None,),
(Path("reeentry_exe_value"),),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def test_map_path(
)

@pytest.mark.parametrize(
argnames=("rules"),
argnames="rules",
argvalues=[
(
[
Expand Down Expand Up @@ -402,7 +402,7 @@ def test_map_path(
mock_establish_named_pipe_connection().close.assert_called_once()

@pytest.mark.parametrize(
argnames=("rules"),
argnames="rules",
argvalues=[
(
[
Expand Down

0 comments on commit 0adc478

Please sign in to comment.