Skip to content

Commit 4fd4ffc

Browse files
committed
refactor: Remove all usage of custom OSName class
* Instead of the custom OSName, use Python idioms. This will be easier to understand for Python developers and tooling that is processing the code. 1. For Windows: os.name == "nt" 2. For POSIX: os.name != "nt" 3. For MacOS: sys.platform == "darwin" 4. For a human-readable OS description: platform.platform() * Comments '# pragma: is-windows' were unclear, it required investigation to learn that this is a coverage directive. Rename the directive to '# pragma: skip-coverage-windows' Signed-off-by: Mark <[email protected]>
1 parent 9648e75 commit 4fd4ffc

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+297
-326
lines changed

pyproject.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,8 @@ source = [
176176
]
177177

178178
[tool.coverage.coverage_conditional_plugin.rules]
179-
is-windows = "sys_platform == 'win32'"
180-
is-posix = "sys_platform != 'win32'"
179+
skip-coverage-windows = "sys_platform == 'win32'"
180+
skip-coverage-posix = "sys_platform != 'win32'"
181181

182182
[tool.coverage.report]
183183
show_missing = true

src/openjd/adaptor_runtime/_background/backend_runner.py

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,21 @@
1010
from threading import Thread, Event
1111
import traceback
1212
from types import FrameType
13-
from typing import Callable, List, Optional, Union
13+
from typing import Callable, List, Optional
1414

1515
from .server_response import ServerResponseGenerator
16-
from .._osname import OSName
1716
from ..adaptors import AdaptorRunner
18-
from .._http import SocketPaths
1917
from .._utils import secure_open
2018

21-
if OSName.is_posix():
22-
from .http_server import BackgroundHTTPServer
23-
if OSName.is_windows():
19+
if os.name == "nt": # pragma: skip-coverage-posix
2420
from ...adaptor_runtime_client.named_pipe.named_pipe_helper import NamedPipeHelper
25-
from .backend_named_pipe_server import WinBackgroundNamedPipeServer
21+
from .backend_named_pipe_server import (
22+
WinBackgroundNamedPipeServer,
23+
WinBackgroundNamedPipeServer as BackgroundServerType,
24+
)
25+
else: # pragma: skip-coverage-windows
26+
from .._http import SocketPaths
27+
from .http_server import BackgroundHTTPServer, BackgroundHTTPServer as BackgroundServerType # type: ignore
2628
from .log_buffers import LogBuffer
2729
from .model import ConnectionSettings
2830
from .model import DataclassJSONEncoder
@@ -46,12 +48,12 @@ def __init__(
4648
self._connection_file_path = connection_file_path
4749

4850
self._log_buffer = log_buffer
49-
self._server: Optional[Union[BackgroundHTTPServer, WinBackgroundNamedPipeServer]] = None
51+
self._server: Optional[BackgroundServerType] = None
5052
signal.signal(signal.SIGINT, self._sigint_handler)
51-
if OSName.is_posix(): # pragma: is-windows
52-
signal.signal(signal.SIGTERM, self._sigint_handler)
53-
else: # pragma: is-posix
53+
if os.name == "nt": # pragma: skip-coverage-posix
5454
signal.signal(signal.SIGBREAK, self._sigint_handler) # type: ignore[attr-defined]
55+
else: # pragma: skip-coverage-windows
56+
signal.signal(signal.SIGTERM, self._sigint_handler)
5557

5658
def _sigint_handler(self, signum: int, frame: Optional[FrameType]) -> None:
5759
"""
@@ -81,24 +83,24 @@ def run(self, *, on_connection_file_written: List[Callable[[], None]] | None = N
8183
_logger.info("Running in background daemon mode.")
8284
shutdown_event: Event = Event()
8385

84-
if OSName.is_posix(): # pragma: is-windows
85-
server_path = SocketPaths.for_os().get_process_socket_path(
86+
if os.name == "nt": # pragma: skip-coverage-posix
87+
server_path = NamedPipeHelper.generate_pipe_name("AdaptorNamedPipe")
88+
else: # pragma: skip-coverage-windows
89+
server_path = SocketPaths.get_process_socket_path(
8690
".openjd_adaptor_runtime",
8791
create_dir=True,
8892
)
89-
else: # pragma: is-posix
90-
server_path = NamedPipeHelper.generate_pipe_name("AdaptorNamedPipe")
9193

9294
try:
93-
if OSName.is_windows(): # pragma: is-posix
95+
if os.name == "nt": # pragma: skip-coverage-posix
9496
self._server = WinBackgroundNamedPipeServer(
9597
server_path,
9698
self._adaptor_runner,
9799
shutdown_event=shutdown_event,
98100
log_buffer=self._log_buffer,
99101
)
100-
else: # pragma: is-windows
101-
self._server = BackgroundHTTPServer(
102+
else: # pragma: skip-coverage-windows
103+
self._server = BackgroundHTTPServer( # type: ignore
102104
server_path,
103105
self._adaptor_runner,
104106
shutdown_event=shutdown_event,
@@ -153,8 +155,8 @@ def run(self, *, on_connection_file_written: List[Callable[[], None]] | None = N
153155
# NamedPipe servers are managed by Named Pipe File System it is not a regular file.
154156
# Once all handles are closed, the system automatically cleans up the named pipe.
155157
files_for_deletion = [self._connection_file_path]
156-
if OSName.is_posix(): # pragma: is-windows
157-
files_for_deletion.append(server_path)
158+
if os.name != "nt": # pragma: skip-coverage-windows
159+
files_for_deletion.append(server_path) # type: ignore
158160
for path in files_for_deletion:
159161
try:
160162
os.remove(path)

src/openjd/adaptor_runtime/_background/frontend_runner.py

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
from types import ModuleType
2121
from typing import Optional, Callable, Dict
2222

23-
from .._osname import OSName
2423
from ..process._logging import _ADAPTOR_OUTPUT_LEVEL
2524
from .._utils._constants import _OPENJD_ENV_STDOUT_PREFIX, _OPENJD_ADAPTOR_SOCKET_ENV
2625
from .loaders import ConnectionSettingsFileLoader
@@ -36,7 +35,7 @@
3635

3736
_FRONTEND_RUNNER_REQUEST_TIMEOUT: float = 5.0
3837

39-
if OSName.is_windows():
38+
if os.name == "nt": # pragma: skip-coverage-posix
4039
from ...adaptor_runtime_client.named_pipe.named_pipe_helper import NamedPipeHelper
4140
import pywintypes
4241

@@ -78,10 +77,10 @@ def __init__(
7877

7978
self._canceled = Event()
8079
signal.signal(signal.SIGINT, self._sigint_handler)
81-
if OSName.is_posix(): # pragma: is-windows
82-
signal.signal(signal.SIGTERM, self._sigint_handler)
83-
else: # pragma: is-posix
80+
if os.name == "nt": # pragma: skip-coverage-posix
8481
signal.signal(signal.SIGBREAK, self._sigint_handler) # type: ignore[attr-defined]
82+
else: # pragma: skip-coverage-windows
83+
signal.signal(signal.SIGTERM, self._sigint_handler)
8584

8685
def init(
8786
self,
@@ -270,7 +269,7 @@ def _heartbeat(self, ack_id: str | None = None) -> HeartbeatResponse:
270269
"""
271270
params: dict[str, str] | None = {"ack_id": ack_id} if ack_id else None
272271
response = self._send_request("GET", "/heartbeat", params=params)
273-
body = json.load(response.fp) if OSName.is_posix() else json.loads(response["body"]) # type: ignore
272+
body = json.loads(response["body"]) if os.name == "nt" else json.load(response.fp) # type: ignore
274273
return DataclassMapper(HeartbeatResponse).map(body)
275274

276275
def _heartbeat_until_state_complete(self, state: AdaptorState) -> None:
@@ -330,7 +329,7 @@ def _send_request(
330329
"Connection settings are required to send requests, but none were provided"
331330
)
332331

333-
if OSName.is_windows(): # pragma: is-posix
332+
if os.name == "nt": # pragma: skip-coverage-posix
334333
if params:
335334
# This is used for aligning to the Linux's behavior in order to reuse the code in handler.
336335
# In linux, query string params will always be put in a list.
@@ -353,7 +352,7 @@ def _send_request(
353352
_logger.error(f"Failed to send {path} request: {e}")
354353
raise
355354
return response
356-
else: # pragma: is-windows
355+
else: # pragma: skip-coverage-windows
357356
return self._send_linux_request(
358357
method,
359358
path,
@@ -368,7 +367,7 @@ def _send_linux_request(
368367
*,
369368
params: dict | None = None,
370369
json_body: dict | None = None,
371-
) -> http_client.HTTPResponse: # pragma: is-windows
370+
) -> http_client.HTTPResponse: # pragma: skip-coverage-windows
372371
if not self.connection_settings:
373372
raise ConnectionSettingsNotProvidedError(
374373
"Connection settings are required to send requests, but none were provided"

src/openjd/adaptor_runtime/_entrypoint.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import contextlib
66
import logging
77
import os
8+
import platform
89
import signal
910
import sys
1011
import tempfile
@@ -40,7 +41,6 @@
4041
RuntimeConfiguration,
4142
ConfigurationManager,
4243
)
43-
from ._osname import OSName
4444
from ._utils._constants import _OPENJD_ADAPTOR_SOCKET_ENV, _OPENJD_LOG_REGEX
4545
from ._utils._logging import (
4646
ConditionalFormatter,
@@ -84,7 +84,7 @@
8484
_DIR = os.path.dirname(os.path.realpath(__file__))
8585
# Keyword args to init the ConfigurationManager for the runtime.
8686
_ENV_CONFIG_PATH_PREFIX = "RUNTIME_CONFIG_PATH"
87-
_system_config_path_prefix = "/etc" if OSName.is_posix() else os.environ["PROGRAMDATA"]
87+
_system_config_path_prefix = os.environ["PROGRAMDATA"] if os.name == "nt" else "/etc"
8888
_system_config_path = os.path.abspath(
8989
os.path.join(
9090
_system_config_path_prefix,
@@ -226,7 +226,7 @@ def _init_config(self) -> None:
226226
raise
227227
except NotImplementedError as e:
228228
_logger.warning(
229-
f"The current system ({OSName()}) is not supported for runtime "
229+
f"The current system ({platform.platform()}) is not supported for runtime "
230230
f"configuration. Only the default configuration will be loaded. Full error: {e}"
231231
)
232232
# The above call to build_config() would have already successfully retrieved the
@@ -354,10 +354,10 @@ def _handle_run(
354354
self._adaptor_runner = AdaptorRunner(adaptor=adaptor)
355355
# To be able to handle cancelation via signals
356356
signal.signal(signal.SIGINT, self._sigint_handler)
357-
if OSName.is_posix(): # pragma: is-windows
358-
signal.signal(signal.SIGTERM, self._sigint_handler)
359-
else: # pragma: is-posix
357+
if os.name == "nt": # pragma: skip-coverage-posix
360358
signal.signal(signal.SIGBREAK, self._sigint_handler) # type: ignore[attr-defined]
359+
else: # pragma: skip-coverage-windows
360+
signal.signal(signal.SIGTERM, self._sigint_handler)
361361
try:
362362
self._adaptor_runner._start()
363363
self._adaptor_runner._run(integration_data.run_data)

src/openjd/adaptor_runtime/_http/request_handler.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
from dataclasses import dataclass
1313
from http import HTTPStatus, server
1414
from typing import Any, Callable, Type
15+
import sys
1516

16-
from .._osname import OSName
1717
from .exceptions import UnsupportedPlatformException
1818

1919
_logger = logging.getLogger(__name__)
@@ -125,8 +125,8 @@ def _authenticate(self) -> bool:
125125
peercred_opt_level: Any
126126
peercred_opt: Any
127127
cred_cls: Any
128-
if OSName.is_macos(): # pragma: no cover
129-
# SOL_LOCAL is not defined in Python's socket module, need to hardcode it
128+
if sys.platform == "darwin":
129+
# On MacOS, SOL_LOCAL is not defined in Python's socket module, need to hardcode it
130130
# source: https://github.com/apple-oss-distributions/xnu/blob/1031c584a5e37aff177559b9f69dbd3c8c3fd30a/bsd/sys/un.h#L85
131131
peercred_opt_level = 0 # type: ignore[attr-defined]
132132
peercred_opt = socket.LOCAL_PEERCRED # type: ignore[attr-defined]

src/openjd/adaptor_runtime/_http/sockets.py

Lines changed: 13 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@
55
import abc
66
import os
77
import stat
8+
import sys
89
import tempfile
10+
from typing import Type
911

10-
from .._osname import OSName
1112
from .exceptions import (
12-
UnsupportedPlatformException,
1313
NonvalidSocketPathException,
1414
NoSocketPathFoundException,
1515
)
@@ -18,29 +18,11 @@
1818
_PID_MAX_LENGTH = 7
1919

2020

21-
class SocketPaths(abc.ABC):
21+
class SocketPathsBase(abc.ABC):
2222
"""
2323
Base class for determining the paths for sockets used in the Adaptor Runtime.
2424
"""
2525

26-
@staticmethod
27-
def for_os(osname: OSName = OSName()): # pragma: no cover
28-
"""
29-
Gets the SocketPaths class for a specific OS.
30-
31-
Args:
32-
osname (OSName, optional): The OS to get socket paths for.
33-
Defaults to the current OS.
34-
35-
Raises:
36-
UnsupportedPlatformException: Raised when this class is requested for an unsupported
37-
platform.
38-
"""
39-
klass = _get_socket_paths_cls(osname)
40-
if not klass:
41-
raise UnsupportedPlatformException(osname)
42-
return klass()
43-
4426
def get_process_socket_path(
4527
self,
4628
namespace: str | None = None,
@@ -143,7 +125,7 @@ def verify_socket_path(self, path: str) -> None: # pragma: no cover
143125
pass
144126

145127

146-
class WindowsSocketPaths(SocketPaths):
128+
class WindowsSocketPaths(SocketPathsBase):
147129
"""
148130
Specialization for verifying socket paths on Windows systems.
149131
"""
@@ -153,7 +135,7 @@ def verify_socket_path(self, path: str) -> None:
153135
pass
154136

155137

156-
class UnixSocketPaths(SocketPaths):
138+
class UnixSocketPaths(SocketPathsBase):
157139
"""
158140
Specialization for verifying socket paths on Unix systems.
159141
"""
@@ -215,14 +197,12 @@ def verify_socket_path(self, path: str) -> None:
215197
)
216198

217199

218-
_os_map: dict[str, type[SocketPaths]] = {
219-
OSName.LINUX: LinuxSocketPaths,
220-
OSName.MACOS: MacOSSocketPaths,
221-
OSName.WINDOWS: WindowsSocketPaths,
222-
}
223-
200+
SocketPaths: Type
224201

225-
def _get_socket_paths_cls(
226-
osname: OSName,
227-
) -> type[SocketPaths] | None: # pragma: no cover
228-
return _os_map.get(osname, None)
202+
# Make SocketPaths the correct operating-specific subclass of SocketPathsBase
203+
if sys.platform == "darwin": # pragma: skip-coverage-windows
204+
SocketPaths = MacOSSocketPaths
205+
elif os.name == "nt": # pragma: skip-coverage-posix
206+
SocketPaths = WindowsSocketPaths
207+
else: # pragma: skip-coverage-windows
208+
SocketPaths = LinuxSocketPaths

src/openjd/adaptor_runtime/_named_pipe/named_pipe_request_handler.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
22

33
import json
4+
import os
5+
import platform
46
from typing import TYPE_CHECKING, Dict, List
57

68
if TYPE_CHECKING: # pragma: no cover because pytest will think we should test for this.
@@ -18,8 +20,6 @@
1820
import traceback
1921
from abc import ABC, abstractmethod
2022

21-
from openjd.adaptor_runtime._osname import OSName
22-
2323

2424
_logger = logging.getLogger(__name__)
2525

@@ -42,10 +42,10 @@ def __init__(self, server: "NamedPipeServer", pipe_handle: HANDLE):
4242
Utilized for message read/write operations.
4343
"""
4444
self._handler_type_name = self.__class__.__name__
45-
if not OSName.is_windows():
45+
if os.name != "nt": # pragma: skip-coverage-windows
4646
raise OSError(
4747
f"{self._handler_type_name} can be only used on Windows Operating Systems. "
48-
f"Current Operating System is {OSName._get_os_name()}"
48+
f"Current Operating System is {platform.platform()}"
4949
)
5050
self.server = server
5151
self.pipe_handle = pipe_handle

src/openjd/adaptor_runtime/_named_pipe/named_pipe_server.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
22

33
from __future__ import annotations
4-
import logging
54

5+
import logging
6+
import os
7+
import platform
68
import threading
79
from threading import Event
810

@@ -20,7 +22,6 @@
2022

2123
if TYPE_CHECKING:
2224
from .._named_pipe import ResourceRequestHandler
23-
from .._osname import OSName
2425

2526
import win32pipe
2627
import win32file
@@ -83,10 +84,10 @@ def __init__(self, pipe_name: str, shutdown_event: Event): # pragma: no cover
8384
shutdown_event (Event): An Event used for signaling server shutdown.
8485
"""
8586
self._server_type_name = self.__class__.__name__
86-
if not OSName.is_windows():
87+
if os.name != "nt": # pragma: skip-coverage-windows
8788
raise OSError(
8889
f"{self._server_type_name} can be only used on Windows Operating Systems. "
89-
f"Current Operating System is {OSName._get_os_name()}"
90+
f"Current Operating System is {platform.platform()}"
9091
)
9192
self._named_pipe_instances: List[HANDLE] = []
9293
self._pipe_name = pipe_name

0 commit comments

Comments
 (0)