From 6aaad8ea22af0664cd96c01215f8c778dcc2106a Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Sat, 18 Jan 2025 21:36:21 +0100 Subject: [PATCH] Chore: A few updates from code review etc. --- responder/__init__.py | 2 +- responder/api.py | 10 ++++++++- responder/ext/cli.py | 4 +++- responder/util/cmd.py | 47 ++++++++++++++++++++++++++++++++++++------- setup.py | 9 ++++++++- tests/test_cli.py | 2 +- 6 files changed, 62 insertions(+), 12 deletions(-) diff --git a/responder/__init__.py b/responder/__init__.py index dcbaeae9..7850655c 100644 --- a/responder/__init__.py +++ b/responder/__init__.py @@ -2,7 +2,7 @@ Responder - a familiar HTTP Service Framework. This module exports the core functionality of the Responder framework, -including the API, Request, Response classes and CLI interface. +including the API, Request, and Response classes. """ from . import ext diff --git a/responder/api.py b/responder/api.py index 25e494c1..ab75aed3 100644 --- a/responder/api.py +++ b/responder/api.py @@ -2,7 +2,6 @@ from pathlib import Path import uvicorn -from starlette.exceptions import ExceptionMiddleware from starlette.middleware.cors import CORSMiddleware from starlette.middleware.errors import ServerErrorMiddleware from starlette.middleware.gzip import GZipMiddleware @@ -11,6 +10,15 @@ from starlette.middleware.trustedhost import TrustedHostMiddleware from starlette.testclient import TestClient +# Python 3.7+ +try: + from starlette.middleware.exceptions import ExceptionMiddleware +# Python 3.6 +except ImportError: + from starlette.exceptions import ( # type: ignore[attr-defined,no-redef] + ExceptionMiddleware, + ) + from . import status_codes from .background import BackgroundQueue from .formats import get_formats diff --git a/responder/ext/cli.py b/responder/ext/cli.py index 6f7908b8..95b22f04 100644 --- a/responder/ext/cli.py +++ b/responder/ext/cli.py @@ -69,12 +69,14 @@ def cli() -> None: sys.exit(1) npm_cmd = "npm.cmd" if platform.system() == "Windows" else "npm" try: - # # S603, S607 are addressed by validating the target directory. + logger.info("Starting frontend asset build") + # S603, S607 are addressed by validating the target directory. subprocess.check_call( # noqa: S603, S607 [npm_cmd, "run", "build"], cwd=target_path, timeout=300, ) + logger.info("Frontend asset build completed successfully") except FileNotFoundError: logger.error("npm not found. Please install Node.js and npm.") sys.exit(1) diff --git a/responder/util/cmd.py b/responder/util/cmd.py index 8efd7440..25eaff76 100644 --- a/responder/util/cmd.py +++ b/responder/util/cmd.py @@ -4,6 +4,7 @@ # 1. Only execute the 'responder' binary from PATH # 2. Validate all user inputs before passing to subprocess # 3. Use Path.resolve() to prevent path traversal +import functools import logging import os import shutil @@ -20,10 +21,19 @@ class ResponderProgram: """ - Provide full path to the `responder` program. + Utility class for managing Responder program execution. + + This class provides methods for: + - Locating the responder executable in PATH + - Building frontend assets using npm + + Example: + >>> program_path = ResponderProgram.path() + >>> build_status = ResponderProgram.build(Path("app_dir")) """ @staticmethod + @functools.lru_cache(maxsize=None) def path(): name = "responder" if sys.platform == "win32": @@ -105,6 +115,13 @@ def __init__(self, target: str, port: int = 5042, limit_max_requests: int = None ): raise ValueError("limit_max_requests must be a positive integer if specified") + # Check if port is available. + try: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.bind(("localhost", port)) + except OSError as ex: + raise ValueError(f"Port {port} is already in use") from ex + # Instance variables after validation. self.target = target self.port = port @@ -114,6 +131,7 @@ def __init__(self, target: str, port: int = 5042, limit_max_requests: int = None # Allow the thread to be terminated when the main program exits. self.process: subprocess.Popen self.daemon = True + self._process_lock = threading.Lock() # Setup signal handlers. signal.signal(signal.SIGTERM, self._signal_handler) @@ -134,19 +152,27 @@ def run(self): if self.port is not None: env["PORT"] = str(self.port) - self.process = subprocess.Popen( - command, - env=env, - universal_newlines=True, - ) + with self._process_lock: + self.process = subprocess.Popen( + command, + env=env, + universal_newlines=True, + ) self.process.wait() def stop(self): """ - Gracefully stop the process. + Gracefully stop the process (API). """ if self._stopping: return + with self._process_lock: + self._stop() + + def _stop(self): + """ + Gracefully stop the process (impl). + """ self._stopping = True if self.process and self.process.poll() is None: logger.info("Attempting to terminate server process...") @@ -179,6 +205,7 @@ def wait_until_ready(self, timeout=30, request_timeout=1, delay=0.1) -> bool: bool: True if server is ready and accepting connections, False otherwise. """ start_time = time.time() + last_error = None while time.time() - start_time < timeout: if not self.is_running(): if self.process is None: @@ -198,8 +225,14 @@ def wait_until_ready(self, timeout=30, request_timeout=1, delay=0.1) -> bool: socket.gaierror, OSError, ) as ex: + last_error = ex logger.debug(f"Server not ready yet: {ex}") time.sleep(delay) + logger.error( + "Server failed to start within %d seconds. Last error: %s", + timeout, + last_error, + ) return False def is_running(self): diff --git a/setup.py b/setup.py index 4f73b66e..1ec1b419 100644 --- a/setup.py +++ b/setup.py @@ -138,7 +138,14 @@ def run(self): "graphql": ["graphene<3", "graphql-server-core>=1.2,<2"], "openapi": ["apispec>=1.0.0"], "release": ["build", "twine"], - "test": ["flask", "mypy", "pytest", "pytest-cov", "pytest-mock", "pytest-rerunfailures"], + "test": [ + "flask", + "mypy", + "pytest", + "pytest-cov", + "pytest-mock", + "pytest-rerunfailures", + ], }, include_package_data=True, license="Apache 2.0", diff --git a/tests/test_cli.py b/tests/test_cli.py index 054addf9..e878ab9d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -7,7 +7,7 @@ - responder run: Server execution Requirements: -- The `docopt` package must be installed +- The `docopt-ng` package must be installed - Example application must be present at `examples/helloworld.py` - This file should implement a basic HTTP server with a "/hello" endpoint that returns "hello, world!" as response