From af268d7607fb387513e2e37d832c43fc42dac597 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Thu, 6 Jan 2022 00:32:34 +0530 Subject: [PATCH 1/9] Add `--port-file` flag --- README.md | 7 +++++-- proxy/common/constants.py | 1 + proxy/common/flag.py | 7 +++++++ proxy/core/acceptor/listener.py | 13 +++++++++++-- proxy/proxy.py | 12 ++++++++++++ 5 files changed, 36 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 1742496456..6e0e5b2c1a 100644 --- a/README.md +++ b/README.md @@ -2206,7 +2206,7 @@ To run standalone benchmark for `proxy.py`, use the following command from repo usage: -m [-h] [--enable-events] [--enable-conn-pool] [--threadless] [--threaded] [--num-workers NUM_WORKERS] [--local-executor LOCAL_EXECUTOR] [--backlog BACKLOG] - [--hostname HOSTNAME] [--port PORT] + [--hostname HOSTNAME] [--port PORT] [--port-file PORT_FILE] [--unix-socket-path UNIX_SOCKET_PATH] [--num-acceptors NUM_ACCEPTORS] [--version] [--log-level LOG_LEVEL] [--log-file LOG_FILE] [--log-format LOG_FORMAT] @@ -2232,7 +2232,7 @@ usage: -m [-h] [--enable-events] [--enable-conn-pool] [--threadless] [--filtered-url-regex-config FILTERED_URL_REGEX_CONFIG] [--cloudflare-dns-mode CLOUDFLARE_DNS_MODE] -proxy.py v2.4.0rc5.dev31+gc998255 +proxy.py v2.4.0rc6.dev13+ga9b8034.d20220104 options: -h, --help show this help message and exit @@ -2261,6 +2261,9 @@ options: proxy server --hostname HOSTNAME Default: 127.0.0.1. Server IP address. --port PORT Default: 8899. Server port. + --port-file PORT_FILE + Default: None. Save server port numbers. Useful when + using --port=0 ephemeral mode. --unix-socket-path UNIX_SOCKET_PATH Default: None. Unix socket path to use. When provided --host and --port flags are ignored diff --git a/proxy/common/constants.py b/proxy/common/constants.py index 9f6ff46deb..176a08fe7b 100644 --- a/proxy/common/constants.py +++ b/proxy/common/constants.py @@ -114,6 +114,7 @@ def _env_threadless_compliant() -> bool: DEFAULT_PAC_FILE = None DEFAULT_PAC_FILE_URL_PATH = b'/' DEFAULT_PID_FILE = None +DEFAULT_PORT_FILE = None DEFAULT_PLUGINS: List[Any] = [] DEFAULT_PORT = 8899 DEFAULT_SERVER_RECVBUF_SIZE = DEFAULT_BUFFER_SIZE diff --git a/proxy/common/flag.py b/proxy/common/flag.py index f0c7f7c7fb..06783b9a80 100644 --- a/proxy/common/flag.py +++ b/proxy/common/flag.py @@ -363,6 +363,13 @@ def initialize( ), ) + args.port_file = cast( + Optional[str], opts.get( + 'port_file', + args.port_file, + ), + ) + args.proxy_py_data_dir = DEFAULT_DATA_DIRECTORY_PATH os.makedirs(args.proxy_py_data_dir, exist_ok=True) diff --git a/proxy/core/acceptor/listener.py b/proxy/core/acceptor/listener.py index c137502db0..d55962a933 100644 --- a/proxy/core/acceptor/listener.py +++ b/proxy/core/acceptor/listener.py @@ -20,7 +20,7 @@ from typing import Optional, Any from ...common.flag import flags -from ...common.constants import DEFAULT_BACKLOG, DEFAULT_IPV4_HOSTNAME, DEFAULT_PORT +from ...common.constants import DEFAULT_BACKLOG, DEFAULT_IPV4_HOSTNAME, DEFAULT_PORT, DEFAULT_PORT_FILE flags.add_argument( @@ -38,10 +38,19 @@ ) flags.add_argument( - '--port', type=int, default=DEFAULT_PORT, + '--port', + type=int, + default=DEFAULT_PORT, help='Default: 8899. Server port.', ) +flags.add_argument( + '--port-file', + type=str, + default=DEFAULT_PORT_FILE, + help='Default: None. Save server port numbers. Useful when using --port=0 ephemeral mode.', +) + flags.add_argument( '--unix-socket-path', type=str, diff --git a/proxy/proxy.py b/proxy/proxy.py index 43a90593af..418c0fc8b7 100644 --- a/proxy/proxy.py +++ b/proxy/proxy.py @@ -164,6 +164,7 @@ def setup(self) -> None: # we are listening upon. This is necessary to preserve # the server port when `--port=0` is used. self.flags.port = self.listener._port + self._write_port_file() # Setup EventManager if self.flags.enable_events: logger.info('Core Event enabled') @@ -204,6 +205,7 @@ def shutdown(self) -> None: self.event_manager.shutdown() assert self.listener self.listener.shutdown() + self._delete_port_file() self._delete_pid_file() @property @@ -221,6 +223,16 @@ def _delete_pid_file(self) -> None: and os.path.exists(self.flags.pid_file): os.remove(self.flags.pid_file) + def _write_port_file(self) -> None: + if self.flags.port_file: + with open(self.flags.port_file, 'wb') as port_file: + port_file.write(bytes_(self.flags.port)) + + def _delete_port_file(self) -> None: + if self.flags.port_file \ + and os.path.exists(self.flags.port_file): + os.remove(self.flags.port_file) + def _register_signals(self) -> None: # TODO: Handle SIGINFO, SIGUSR1, SIGUSR2 signal.signal(signal.SIGINT, self._handle_exit_signal) From 514161afdfce947ee033b348922a370f329c4709 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Thu, 6 Jan 2022 00:35:44 +0530 Subject: [PATCH 2/9] Use `--port-file` flag for integration tests using `get_available_port` --- tests/integration/test_integration.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index 5894e13963..623dda0a85 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -17,7 +17,6 @@ from typing import Generator, Any from proxy.common.constants import IS_WINDOWS -from proxy.common.utils import get_available_port # FIXME: Ignore is necessary for as long as pytest hasn't figured out @@ -34,14 +33,15 @@ def proxy_py_subprocess(request: Any) -> Generator[int, None, None]: After the testing is over, tear it down. """ - port = get_available_port() proxy_cmd = ( 'python', '-m', 'proxy', '--hostname', '127.0.0.1', - '--port', str(port), + '--port', '0', + '--port-file', '/tmp/proxy.port', '--enable-web-server', ) + tuple(request.param.split()) proxy_proc = Popen(proxy_cmd) + port = int(Path('/tmp/proxy.port').read_text()) try: yield port finally: From e9da2accebe0383f3ab26558cefd2065e1c694e0 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Thu, 6 Jan 2022 00:40:36 +0530 Subject: [PATCH 3/9] Use temp dir --- tests/integration/test_integration.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index 623dda0a85..bdd39eb4b8 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -11,6 +11,7 @@ Test the simplest proxy use scenario for smoke. """ import pytest +import tempfile from pathlib import Path from subprocess import check_output, Popen @@ -33,15 +34,16 @@ def proxy_py_subprocess(request: Any) -> Generator[int, None, None]: After the testing is over, tear it down. """ + port_file = Path(tempfile.gettempdir()) / "proxy.port" proxy_cmd = ( 'python', '-m', 'proxy', '--hostname', '127.0.0.1', '--port', '0', - '--port-file', '/tmp/proxy.port', + '--port-file', str(port_file), '--enable-web-server', ) + tuple(request.param.split()) proxy_proc = Popen(proxy_cmd) - port = int(Path('/tmp/proxy.port').read_text()) + port = int(port_file.read_text()) try: yield port finally: From 48802ec37e0f6d1070b5a5d773f8734b2a08a58d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 5 Jan 2022 19:11:35 +0000 Subject: [PATCH 4/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/integration/test_integration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index bdd39eb4b8..73fac2720e 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -34,7 +34,7 @@ def proxy_py_subprocess(request: Any) -> Generator[int, None, None]: After the testing is over, tear it down. """ - port_file = Path(tempfile.gettempdir()) / "proxy.port" + port_file = Path(tempfile.gettempdir()) / 'proxy.port' proxy_cmd = ( 'python', '-m', 'proxy', '--hostname', '127.0.0.1', From d519dcc2f4dfe1de44c1244a54ec03035a7171c4 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Thu, 6 Jan 2022 00:47:27 +0530 Subject: [PATCH 5/9] Fix `base_klass` variable related lint issues --- proxy/common/plugins.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/proxy/common/plugins.py b/proxy/common/plugins.py index 1c3ec84595..2349e6ec9c 100644 --- a/proxy/common/plugins.py +++ b/proxy/common/plugins.py @@ -73,12 +73,12 @@ def load( mro = list(inspect.getmro(klass)) # Find the base plugin class that # this plugin_ is implementing - found = False - for base_klass in mro: - if bytes_(base_klass.__name__) in p: - found = True + base_klass = None + for k in mro: + if bytes_(k.__name__) in p: + base_klass = k break - if not found: + if base_klass is None: raise ValueError('%s is NOT a valid plugin' % text_(plugin_)) if klass not in p[bytes_(base_klass.__name__)]: p[bytes_(base_klass.__name__)].append(klass) From dc2e4fe4288e18f09006dada2d07aa512c956f5d Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Thu, 6 Jan 2022 00:51:05 +0530 Subject: [PATCH 6/9] Fix main tests --- tests/test_main.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_main.py b/tests/test_main.py index 380ffd65b1..3f8816edda 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -15,7 +15,7 @@ from unittest import mock from proxy.proxy import main, entry_point -from proxy.common.constants import _env_threadless_compliant # noqa: WPS450 +from proxy.common.constants import DEFAULT_PORT_FILE, _env_threadless_compliant # noqa: WPS450 from proxy.common.utils import bytes_ from proxy.common.constants import DEFAULT_ENABLE_DASHBOARD, DEFAULT_LOCAL_EXECUTOR, DEFAULT_LOG_LEVEL, DEFAULT_LOG_FILE @@ -69,6 +69,7 @@ def mock_default_args(mock_args: mock.Mock) -> None: mock_args.enable_dashboard = DEFAULT_ENABLE_DASHBOARD mock_args.work_klass = DEFAULT_WORK_KLASS mock_args.local_executor = int(DEFAULT_LOCAL_EXECUTOR) + mock_args.port_file = DEFAULT_PORT_FILE @mock.patch('os.remove') @mock.patch('os.path.exists') @@ -96,6 +97,7 @@ def test_entry_point( mock_initialize.return_value.local_executor = 0 mock_initialize.return_value.enable_events = False mock_initialize.return_value.pid_file = pid_file + mock_initialize.return_value.port_file = None entry_point() mock_event_manager.assert_not_called() mock_listener.assert_called_once_with( @@ -143,6 +145,7 @@ def test_main_with_no_flags( mock_sleep.side_effect = KeyboardInterrupt() mock_initialize.return_value.local_executor = 0 mock_initialize.return_value.enable_events = False + mock_initialize.return_value.port_file = None main() mock_event_manager.assert_not_called() mock_listener.assert_called_once_with( @@ -183,6 +186,7 @@ def test_enable_events( mock_sleep.side_effect = KeyboardInterrupt() mock_initialize.return_value.local_executor = 0 mock_initialize.return_value.enable_events = True + mock_initialize.return_value.port_file = None main() mock_event_manager.assert_called_once() mock_event_manager.return_value.setup.assert_called_once() From 063b4410d7eb28a6ef6d057920815c24b6f879f7 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Thu, 6 Jan 2022 01:10:50 +0530 Subject: [PATCH 7/9] Fix integration --- tests/integration/test_integration.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index 73fac2720e..eb87625aa4 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -10,6 +10,7 @@ Test the simplest proxy use scenario for smoke. """ +import time import pytest import tempfile @@ -42,10 +43,13 @@ def proxy_py_subprocess(request: Any) -> Generator[int, None, None]: '--port-file', str(port_file), '--enable-web-server', ) + tuple(request.param.split()) + print(' '.join(proxy_cmd)) proxy_proc = Popen(proxy_cmd) - port = int(port_file.read_text()) + # Needed because port file might not be available immediately + while not port_file.exists(): + time.sleep(1) try: - yield port + yield int(port_file.read_text()) finally: proxy_proc.terminate() proxy_proc.wait() From c2893da25a632d4c590a4495644314f6c6f5eb6a Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Thu, 6 Jan 2022 01:43:18 +0530 Subject: [PATCH 8/9] Use timeout when terminating proc --- proxy/proxy.py | 2 +- tests/integration/test_integration.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/proxy/proxy.py b/proxy/proxy.py index 418c0fc8b7..42830f2071 100644 --- a/proxy/proxy.py +++ b/proxy/proxy.py @@ -239,7 +239,7 @@ def _register_signals(self) -> None: signal.signal(signal.SIGTERM, self._handle_exit_signal) if not IS_WINDOWS: signal.signal(signal.SIGHUP, self._handle_exit_signal) - # TODO: SIGQUIT is ideally meant for terminate with core dumps + # TODO: SIGQUIT is ideally meant to terminate with core dumps signal.signal(signal.SIGQUIT, self._handle_exit_signal) @staticmethod diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index eb87625aa4..b8ea9b2136 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -43,7 +43,6 @@ def proxy_py_subprocess(request: Any) -> Generator[int, None, None]: '--port-file', str(port_file), '--enable-web-server', ) + tuple(request.param.split()) - print(' '.join(proxy_cmd)) proxy_proc = Popen(proxy_cmd) # Needed because port file might not be available immediately while not port_file.exists(): @@ -52,7 +51,7 @@ def proxy_py_subprocess(request: Any) -> Generator[int, None, None]: yield int(port_file.read_text()) finally: proxy_proc.terminate() - proxy_proc.wait() + proxy_proc.wait(timeout=10) # FIXME: Ignore is necessary for as long as pytest hasn't figured out From 5871e122170f5c04ae809ab086f9e56747ae11af Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Thu, 6 Jan 2022 01:54:47 +0530 Subject: [PATCH 9/9] Skip integration on win instead of xmark --- tests/integration/test_integration.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index b8ea9b2136..66307d9ccc 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -51,7 +51,7 @@ def proxy_py_subprocess(request: Any) -> Generator[int, None, None]: yield int(port_file.read_text()) finally: proxy_proc.terminate() - proxy_proc.wait(timeout=10) + proxy_proc.wait() # FIXME: Ignore is necessary for as long as pytest hasn't figured out @@ -69,10 +69,9 @@ def proxy_py_subprocess(request: Any) -> Generator[int, None, None]: ), indirect=True, ) # type: ignore[misc] -@pytest.mark.xfail( +@pytest.mark.skipif( IS_WINDOWS, reason='OSError: [WinError 193] %1 is not a valid Win32 application', - raises=OSError, ) # type: ignore[misc] def test_integration(proxy_py_subprocess: int) -> None: """An acceptance test using ``curl`` through proxy.py."""