Add --port-file flag (#942)

* Add `--port-file` flag

* Use `--port-file` flag for integration tests using `get_available_port`

* Use temp dir

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fix `base_klass` variable related lint issues

* Fix main tests

* Fix integration

* Use timeout when terminating proc

* Skip integration on win instead of xmark

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: abhinavsingh

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

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

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)
 

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)

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,

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,13 +223,23 @@ 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)
         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

tests/integration/test_integration.py

@@ -10,14 +10,15 @@
 
     Test the simplest proxy use scenario for smoke.
 """
+import time
 import pytest
+import tempfile
 
 from pathlib import Path
 from subprocess import check_output, Popen
 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,16 +35,20 @@ def proxy_py_subprocess(request: Any) -> Generator[int, None, None]:
 
     After the testing is over, tear it down.
     """
-    port = get_available_port()
+    port_file = Path(tempfile.gettempdir()) / 'proxy.port'
     proxy_cmd = (
         'python', '-m', 'proxy',
         '--hostname', '127.0.0.1',
-        '--port', str(port),
+        '--port', '0',
+        '--port-file', str(port_file),
         '--enable-web-server',
     ) + tuple(request.param.split())
     proxy_proc = Popen(proxy_cmd)
+    # 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()
@@ -64,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."""

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()